Skip to content

Conversation

dcog989
Copy link
Contributor

@dcog989 dcog989 commented Sep 22, 2025

Replacement for BrowserBookmark Plugin

This is a new plugin to provide browser bookmarks and favicon functionality. It replaces the existing 'BrowserBookmark` plugin. It improves performance, extensibility, and UX over the original.

Favicons are now available without the need to restart the app, with instant updates for Chrome/ium browsers and a three-hour refresh cycle for Firefox.

Features & Enhancements

  • Favicon Handling:

    • Local Extraction: The plugin first extracts favicons from the browser's local database. This is a fast / low cost function so the previous warning that this might be slow has been removed.
    • Web Fetching (Optional): An optional setting fetches missing favicons from the web. We use a smart fetch favicon.ico / parse the HTML function, processed on a background thread.
    • Format Support: ICO / PNG / WEBP / SVG are supported.
  • Real-time & Periodic Updates:

    • Chromium: Changes to bookmarks in Chromium-based browsers are detected real-time using a FileSystemWatcher with debouncing to prevent excessive reloads.
    • Firefox: To avoid performance issues from watching the frequently-updated places.sqlite file, Firefox bookmarks are now reloaded on a periodic 3-hour timer.

Architectural & Performance

  • Fully Asynchronous Loading: The entire bookmark and favicon loading pipeline is asynchronous (async/await).

  • Parallel Processing: Favicon processing is parallelized to significantly speed up the initial caching process.

  • Separation of Concerns: The logic is cleanly divided:

    • BookmarkLoaderService: Handles discovering and loading bookmarks from different browser types.
    • BookmarkWatcherService: Manages real-time file system monitoring for Chromium browsers.
    • FaviconService: Responsible for all favicon extraction, fetching, and caching.
    • BrowserDetector: Contains the logic to automatically identify a browser's engine from its directory path.

UX & UI Improvements

  • Automatic Engine Detection: The user no longer needs to specify a custom browser engine, we auto-detect.

A complete redesign focused on performance / low system impact / good separation of logic for easy maintenance.

## Architectural

- all asynchronous, service-oriented design.
- distinct services for loading bookmarks, handling favicons, monitoring files

## Performance

- Faster Startup: bookmarks are loaded first and asynchronously for 'instant' search. The slower favicon processing is then completed in the background without blocking the UI.
- Optimized Firefox: avoids the slow / high disk `places.sqlite` file copy in most instances, so much quicker loading of Firefox bookmarks.
- Real-Time Updates: the hyperactive rescanning issue has been fixed. File monitoring for Chromium is now low-overhead with immediate update. The unstable monitoring (of `places.sqlite`) for Firefox has been removed and replaced with periodic check.
@prlabeler prlabeler bot added the enhancement New feature or request label Sep 22, 2025
@github-actions github-actions bot added this to the 2.1.0 milestone Sep 22, 2025
Copy link

gitstream-cm bot commented Sep 22, 2025

🥷 Code experts: jjw24

jjw24 has most 🧠 knowledge in the files.

See details

Flow.Launcher.sln

Knowledge based on git-blame:
jjw24: 25%

✨ Comment /gs review for LinearB AI review. Learn how to automate it here.

Copy link

gitstream-cm bot commented Sep 22, 2025

Be a legend 🏆 by adding a before and after screenshot of the changes you made, especially if they are around UI/UX.

@Jack251970
Copy link
Member

Jack251970 commented Sep 22, 2025

@dcog989 Why did you change the directory of the plugin from Flow.Launcher.Plugin.Bookmarks to Flow.Launcher.Plugin.Bookmark? I just think it is unnecessary and I do not recommend to do that.

@Jack251970 Jack251970 requested a review from Copilot September 22, 2025 13:00
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR replaces the existing BrowserBookmark plugin with a completely rewritten BrowserBookmarks plugin that improves performance, extensibility, and UX. The new plugin introduces automatic browser engine detection, real-time bookmark updates for Chromium browsers, and enhanced favicon handling with optional web fetching.

Key Changes:

  • Complete plugin rewrite with async/await architecture and better separation of concerns
  • Automatic browser engine detection eliminates manual configuration requirement
  • Real-time file monitoring for Chromium browsers with periodic updates for Firefox
  • Enhanced favicon support with local extraction and optional web fetching

Reviewed Changes

Copilot reviewed 39 out of 66 changed files in this pull request and generated 4 comments.

File Description
Plugins/Flow.Launcher.Plugin.BrowserBookmarks/ New plugin implementation with improved architecture
Plugins/Flow.Launcher.Plugin.BrowserBookmark/ Removed old plugin files
Flow.Launcher.sln Updated solution to reference new plugin

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

{DB90F671-D861-46BB-93A3-F1304F5BA1C5}.Release|x64.Build.0 = Release|Any CPU
{DB90F671-D861-46BB-93A3-F1304F5BA1C5}.Release|x86.ActiveCfg = Release|Any CPU
{DB90F671-D861-46BB-93A3-F1304F5BA1C5}.Release|x86.Build.0 = Release|Any CPU
{FF742965-9A80-41A5-B042-D6C7D3A21708}.Debug|Any CPU.ActiveCfg = Debug|Any CPU
Copy link
Preview

Copilot AI Sep 22, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing Build.0 configuration for Debug|Any CPU platform. This line should include '.Build.0 = Debug|Any CPU' similar to other configurations to ensure the project builds in Debug mode.

Suggested change
{FF742965-9A80-41A5-B042-D6C7D3A21708}.Debug|Any CPU.ActiveCfg = Debug|Any CPU
{FF742965-9A80-41A5-B042-D6C7D3A21708}.Debug|Any CPU.ActiveCfg = Debug|Any CPU
{FF742965-9A80-41A5-B042-D6C7D3A21708}.Debug|Any CPU.Build.0 = Debug|Any CPU

Copilot uses AI. Check for mistakes.

@dcog989
Copy link
Contributor Author

dcog989 commented Sep 22, 2025

Why did you change the directory of the plugin

It's a new plugin. All code is new apart from some reused XAML. Is this critical?

@Jack251970
Copy link
Member

Jack251970 commented Sep 23, 2025

Why did you change the directory of the plugin

It's a new plugin. All code is new apart from some reused XAML. Is this critical?

A very noticeable problem is that users will lose their settings and caches of old Bookmark plugin since you change the namespace. And I am not sure if Crowdin can work here. Additionally, we need to check related Github Actions & CI/workflows.

namespace Flow.Launcher.Plugin.BrowserBookmarks.Models;

// A base class that implements INotifyPropertyChanged for view models.
public abstract class BaseModel : INotifyPropertyChanged
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why we do not use this?

public class BaseModel : INotifyPropertyChanged

Copy link
Contributor

coderabbitai bot commented Sep 23, 2025

Note

Other AI code review bot(s) detected

CodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review.

📝 Walkthrough

Walkthrough

Replaces the old BrowserBookmark plugin with a new BrowserBookmarks plugin. Updates the solution to reference the new project. Removes all legacy loaders/UI/models and adds a new async loading pipeline, favicon services, watcher service, MVVM settings UI, localization updates, and a new plugin manifest.

Changes

Cohort / File(s) Summary
Solution restructure
Flow.Launcher.sln, Plugins\Flow.Launcher.Plugin.BrowserBookmarks\Flow.Launcher.Plugin.BrowserBookmarks.csproj, Plugins\Flow.Launcher.Plugin.BrowserBookmarks\Flow.Launcher.Plugin.BrowserBookmarks.sln
Added new BrowserBookmarks project, removed old BrowserBookmark reference, aligned configuration mappings and outputs.
Remove legacy BrowserBookmark plugin
Plugins\Flow.Launcher.Plugin.BrowserBookmark\*
Deleted entire old plugin: Main, models, settings, loaders (Chromium/Chrome/Edge/Firefox/custom), helper (FaviconHelper), commands, views, plugin.json.
New plugin core (BrowserBookmarks)
Plugins\Flow.Launcher.Plugin.BrowserBookmarks\Main.cs, ...Models\*.cs, ...Services\*.cs, ...ViewModels\*.cs
Introduced async bookmark loaders (Chromium/Firefox), favicon pipeline (local + web), file watchers with debounce, browser detection, Firefox profile finder, new models (Settings/Bookmark), MVVM view model.
Views (MVVM) and bindings
Plugins\Flow.Launcher.Plugin.BrowserBookmarks\Views\SettingsControl.xaml, ...Views\SettingsControl.xaml.cs, ...Views\CustomBrowserSetting.xaml, ...Views\CustomBrowserSetting.xaml.cs
Replaced code-behind handlers with commands, updated bindings to Settings and view model, added engine detection text and folder browsing, updated class names/namespaces.
Localization updates
Plugins\Flow.Launcher.Plugin.BrowserBookmarks\Languages\*.xaml
en.xaml adds/updates strings (favicons, copy URL, guidance, engine detection). Most other locales have formatting/BOM tweaks; ja.xaml content changed to English values.
Plugin manifest swap
Plugins\Flow.Launcher.Plugin.BrowserBookmarks\plugin.json, Plugins\Flow.Launcher.Plugin.BrowserBookmark\plugin.json
Added new manifest for BrowserBookmarks; removed old manifest.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant FL as Flow Launcher
  participant Plugin as BrowserBookmarks.Main
  participant BLS as BookmarkLoaderService
  participant FWS as BookmarkWatcherService
  participant FVS as FaviconService

  FL->>Plugin: Init(context)
  Plugin->>Plugin: Setup temp dir, load Settings
  Plugin->>BLS: Construct(loaders from Settings)
  Plugin->>FWS: Construct()
  Plugin->>FVS: Construct()
  FWS-->>Plugin: OnBookmarkFileChanged (event)
  Plugin->>Plugin: ReloadDataAsync()
  Plugin->>BLS: LoadBookmarksAsync()
  BLS-->>Plugin: Bookmarks + Discovered files
  Plugin->>FWS: UpdateWatchers(files)
  Plugin->>FVS: ProcessBookmarkFavicons(bookmarks) [async]

  FL->>Plugin: Query(q)
  Plugin->>Plugin: Fuzzy match Name/URL
  Plugin-->>FL: Results (open URL, copy URL ctx)

  rect rgba(200,235,255,0.2)
  note right of Plugin: Firefox periodic reload (if enabled)
  Plugin->>BLS: Load Firefox-only
  BLS-->>Plugin: Firefox bookmarks
  Plugin->>Plugin: Merge & swap cache
  end
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related PRs

Suggested reviewers

  • onesounds
  • jjw24

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Title Check ✅ Passed The provided title "New browser bookmarks plugin" succinctly and accurately summarizes the primary change in the changeset: a replacement/addition of the browser bookmarks plugin with improved favicon and loading behavior. It is concise, on-topic, and contains no noisy file lists or emojis. A reviewer scanning PR history would understand the main intent from this title.
Description Check ✅ Passed The PR description directly describes the replacement of the old BrowserBookmark plugin and summarizes key features and architectural changes (favicon handling, async loading, Chromium file-watching, Firefox periodic reload, and new services), matching the raw summary and objectives. It is on-topic, sufficiently detailed for reviewers, and aligns with the scope of the changeset. Therefore it satisfies this lenient description check.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.
✨ Finishing touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment

Tip

👮 Agentic pre-merge checks are now available in preview!

Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.

  • Built-in checks – Quickly apply ready-made checks to enforce title conventions, require pull request descriptions that follow templates, validate linked issues for compliance, and more.
  • Custom agentic checks – Define your own rules using CodeRabbit’s advanced agentic capabilities to enforce organization-specific policies and workflows. For example, you can instruct CodeRabbit’s agent to verify that API documentation is updated whenever API schema files are modified in a PR. Note: Upto 5 custom checks are currently allowed during the preview period. Pricing for this feature will be announced in a few weeks.

Please see the documentation for more information.

Example:

reviews:
  pre_merge_checks:
    custom_checks:
      - name: "Undocumented Breaking Changes"
        mode: "warning"
        instructions: |
          Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).

Please share your feedback with us on this Discord post.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 17

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (3)
Plugins/Flow.Launcher.Plugin.BrowserBookmarks/Languages/da.xaml (1)

1-34: Incomplete Danish localization - Most strings remain in English.

The Danish resource file contains mostly English strings instead of proper Danish translations. Only three strings on Lines 23-25 are translated to Danish ("Tilføj", "Rediger", "Slet"). All other strings should be properly localized.

-    <system:String x:Key="flowlauncher_plugin_browserbookmark_plugin_name">Browser Bookmarks</system:String>
+    <system:String x:Key="flowlauncher_plugin_browserbookmark_plugin_name">Browser Bogmærker</system:String>

-    <system:String x:Key="flowlauncher_plugin_browserbookmark_plugin_description">Search your browser bookmarks</system:String>
+    <system:String x:Key="flowlauncher_plugin_browserbookmark_plugin_description">Søg i dine browser bogmærker</system:String>

-    <system:String x:Key="flowlauncher_plugin_browserbookmark_enable_favicons">Load favicons (can be time consuming during startup)</system:String>
+    <system:String x:Key="flowlauncher_plugin_browserbookmark_enable_favicons">Indlæs favicons (kan tage tid under opstart)</system:String>

This affects user experience for Danish users who will see mixed English/Danish text in the interface.

Plugins/Flow.Launcher.Plugin.BrowserBookmarks/Models/CustomBrowser.cs (1)

56-67: Add localization for BrowserType.Unknown

Plugins/Flow.Launcher.Plugin.BrowserBookmarks/Languages/en.xaml has no mapping for the EnumLocalizeValue "Unknown" — add the corresponding resource entry (follow the plugin's key naming convention) and sync it to Crowdin.

Flow.Launcher.sln (1)

5-20: CI path mismatch — fix BrowserBookmark vs BrowserBookmarks and confirm plugin ID/migrations

  • .github/workflows/default_plugins.yml references Plugins/Flow.Launcher.Plugin.BrowserBookmark/* and publishes Flow.Launcher.Plugin.BrowserBookmark, but the repo contains Plugins/Flow.Launcher.Plugin.BrowserBookmarks/plugin.json (ID: 0ECADE17459B49F587BF81DC3A125110). Update workflow paths or rename the folder to match.
  • crowdin.yml uses Languages/en.xaml and Properties/Resources.resx (no plugin paths) — Crowdin config appears unaffected.
  • If the rename intentionally changed the plugin identity, ensure plugin.json ID matches the previous ID or implement runtime migration for settings/caches (per Main.cs comment). Also update any Scripts/ or .github/actions references (e.g., .github/actions/spelling/expect.txt contains "browserbookmark").
🧹 Nitpick comments (81)
Plugins/Flow.Launcher.Plugin.BrowserBookmarks/Languages/nb.xaml (7)

9-9: Localize the English fallback ("copy failed") to Norwegian.

Current value is English; inconsistent with the NB locale.

Apply:

-    <system:String x:Key="flowlauncher_plugin_browserbookmark_copy_failed">Failed to set url in clipboard</system:String>
+    <system:String x:Key="flowlauncher_plugin_browserbookmark_copy_failed">Kunne ikke kopiere nettadresse til utklippstavlen</system:String>

31-31: Localize the favicons setting string.

English text in NB file.

Apply:

-    <system:String x:Key="flowlauncher_plugin_browserbookmark_enable_favicons">Load favicons (can be time consuming during startup)</system:String>
+    <system:String x:Key="flowlauncher_plugin_browserbookmark_enable_favicons">Last inn favikoner (kan være tidkrevende ved oppstart)</system:String>

16-16: Fix Norwegian wording ("banen" → "sti").

“Bane” is incorrect for filesystem path.

Apply:

-    <system:String x:Key="flowlauncher_plugin_browserbookmark_settings_setBrowserFromPath">Angi nettleser fra banen:</system:String>
+    <system:String x:Key="flowlauncher_plugin_browserbookmark_settings_setBrowserFromPath">Angi nettleser fra sti:</system:String>

20-20: Clarify intent: load bookmarks, not the browser.

Wording suggests loading the browser itself.

Apply:

-    <system:String x:Key="flowlauncher_plugin_browserbookmark_loadBrowserFrom">Last nettleser fra:</system:String>
+    <system:String x:Key="flowlauncher_plugin_browserbookmark_loadBrowserFrom">Last bokmerker fra:</system:String>

26-26: Prefer “Bla gjennom” for browse action.

More idiomatic button label.

Apply:

-    <system:String x:Key="flowlauncher_plugin_browserbookmark_browseBrowserBookmark">Bla</system:String>
+    <system:String x:Key="flowlauncher_plugin_browserbookmark_browseBrowserBookmark">Bla gjennom</system:String>

30-30: Norwegian possessive for “Brave's”.

Use “Braves motor”.

Apply:

-    <system:String x:Key="flowlauncher_plugin_browserbookmark_guideMessage02">For eksempel: Brave's motor er Chromium; og standardplasseringen av bokmerker er: &quot;%LOCALAPPDATA%\BraveSoftware\Brave-Browser\UserData&quot;. For Firefox-motoren er bokmerkekatalogen brukerdatamappen som inneholder places.sqlite-filen.</system:String>
+    <system:String x:Key="flowlauncher_plugin_browserbookmark_guideMessage02">For eksempel: Braves motor er Chromium; og standardplasseringen av bokmerker er: &quot;%LOCALAPPDATA%\BraveSoftware\Brave-Browser\UserData&quot;. For Firefox-motoren er bokmerkekatalogen brukerdatamappen som inneholder places.sqlite-filen.</system:String>

2-33: Verify resource key prefix vs. plugin rename; confirm Crowdin path updates.

Keys retain “browserbookmark” (singular) prefix while project is “BrowserBookmarks” (plural). Fine if code/XAML still reference these keys, but please verify to avoid missing strings. Also confirm Crowdin/source extraction jobs track the new plugin directory so NB (and other locales) continue to sync.

If helpful, I can provide a grep script to confirm key usage across the new project and check Crowdin config changes.

Plugins/Flow.Launcher.Plugin.BrowserBookmarks/Languages/uk-UA.xaml (2)

31-31: Update favicon setting copy to reflect background/parallel fetching.

Apply:

-    <system:String x:Key="flowlauncher_plugin_browserbookmark_enable_favicons">Завантажити піктограми (може зайняти багато часу під час запуску)</system:String>
+    <system:String x:Key="flowlauncher_plugin_browserbookmark_enable_favicons">Увімкнути значки сайтів; відсутні значки можуть завантажуватися у фоновому режимі</system:String>

9-9: Ukrainian microcopy: use “скопіювати” (not “вставити”) and uppercase “URL”.

Apply:

-    <system:String x:Key="flowlauncher_plugin_browserbookmark_copy_failed">Не вдалося вставити Url-адресу в буфер обміну</system:String>
+    <system:String x:Key="flowlauncher_plugin_browserbookmark_copy_failed">Не вдалося скопіювати URL-адресу до буфера обміну</system:String>
Plugins/Flow.Launcher.Plugin.BrowserBookmarks/Languages/sk.xaml (2)

31-31: Favicon setting text implies slower startup; now favicons load async.

Adjust wording to reflect background/async favicon loading and optional web fetch, not startup latency.


1-1: Byte Order Mark present. Consider removing to reduce diff churn.

The leading BOM before the XML declaration can cause noisy diffs across locales. Prefer UTF‑8 without BOM unless your tooling requires it.

If the repo standardizes on no‑BOM, apply “Save without BOM” to all XAML locale files in this plugin for consistency.

Plugins/Flow.Launcher.Plugin.BrowserBookmarks/Languages/ko.xaml (2)

9-10: Untranslated string and casing (URL) — localize for ko-KR.

Line 9 is English and uses "url" instead of "URL". Recommend Korean translation consistent with other entries.

-    <system:String x:Key="flowlauncher_plugin_browserbookmark_copy_failed">Failed to set url in clipboard</system:String>
+    <system:String x:Key="flowlauncher_plugin_browserbookmark_copy_failed">클립보드에 URL을 복사하지 못했습니다</system:String>

13-16: Several English strings left in Korean resource file — translate for consistency.

Localize these labels to avoid mixed-language UI.

-    <system:String x:Key="flowlauncher_plugin_browserbookmark_settings_openBookmarks">Open bookmarks in:</system:String>
+    <system:String x:Key="flowlauncher_plugin_browserbookmark_settings_openBookmarks">북마크를 열 위치:</system:String>
-    <system:String x:Key="flowlauncher_plugin_browserbookmark_settings_setBrowserFromPath">Set browser from path:</system:String>
+    <system:String x:Key="flowlauncher_plugin_browserbookmark_settings_setBrowserFromPath">경로에서 브라우저 지정:</system:String>
-    <system:String x:Key="flowlauncher_plugin_browserbookmark_others">Others</system:String>
+    <system:String x:Key="flowlauncher_plugin_browserbookmark_others">기타</system:String>
-    <system:String x:Key="flowlauncher_plugin_browserbookmark_enable_favicons">Load favicons (can be time consuming during startup)</system:String>
+    <system:String x:Key="flowlauncher_plugin_browserbookmark_enable_favicons">파비콘 로드(시작 시 시간이 오래 걸릴 수 있음)</system:String>

Also applies to: 27-27, 31-31

Plugins/Flow.Launcher.Plugin.BrowserBookmarks/Languages/cs.xaml (3)

9-9: Untranslated string in Czech locale.

Failed to set url in clipboard is English and “url” casing is inconsistent. Suggest Czech + URL casing.

Apply:

-    <system:String x:Key="flowlauncher_plugin_browserbookmark_copy_failed">Failed to set url in clipboard</system:String>
+    <system:String x:Key="flowlauncher_plugin_browserbookmark_copy_failed">Nepodařilo se zkopírovat adresu URL do schránky</system:String>

31-31: Untranslated + possibly outdated performance note.

Localize and avoid implying startup blocking if work is now async/parallel.

Apply:

-    <system:String x:Key="flowlauncher_plugin_browserbookmark_enable_favicons">Load favicons (can be time consuming during startup)</system:String>
+    <system:String x:Key="flowlauncher_plugin_browserbookmark_enable_favicons">Načítat favikony (první načtení může chvíli trvat)</system:String>

18-19: Consistency: use “URL” casing in Czech strings.

Keep “URL” uppercase for consistency with other entries.

Apply:

-    <system:String x:Key="flowlauncher_plugin_browserbookmark_copyurl_title">Kopírovat URL</system:String>
-    <system:String x:Key="flowlauncher_plugin_browserbookmark_copyurl_subtitle">Zkopírovat adresu URL záložky do schránky</system:String>
+    <system:String x:Key="flowlauncher_plugin_browserbookmark_copyurl_title">Kopírovat URL</system:String>
+    <system:String x:Key="flowlauncher_plugin_browserbookmark_copyurl_subtitle">Zkopírovat adresu URL záložky do schránky</system:String>

(If already consistent across locales, ignore.)

Plugins/Flow.Launcher.Plugin.BrowserBookmarks/Languages/vi.xaml (4)

9-9: Untranslated/error‑case string; fix capitalization and locale terms.

Use Vietnamese and “URL” capitalization; prefer “bộ nhớ tạm” for clipboard.

-    <system:String x:Key="flowlauncher_plugin_browserbookmark_copy_failed">Failed to set url in clipboard</system:String>
+    <system:String x:Key="flowlauncher_plugin_browserbookmark_copy_failed">Không thể sao chép URL vào bộ nhớ tạm</system:String>

18-19: Copy URL strings: capitalize “URL” and localize “clipboard”.

Keeps terminology consistent across vi locale.

-    <system:String x:Key="flowlauncher_plugin_browserbookmark_copyurl_title">Sao chép url</system:String>
-    <system:String x:Key="flowlauncher_plugin_browserbookmark_copyurl_subtitle">Sao chép url của dấu trang vào clipboard</system:String>
+    <system:String x:Key="flowlauncher_plugin_browserbookmark_copyurl_title">Sao chép URL</system:String>
+    <system:String x:Key="flowlauncher_plugin_browserbookmark_copyurl_subtitle">Sao chép URL của dấu trang vào bộ nhớ tạm</system:String>

15-15: UI wording nit: drop “Thêm” for the option label.

“Tab mới” reads more natural in settings.

-    <system:String x:Key="flowlauncher_plugin_browserbookmark_settings_newTab">Thêm Tab mới</system:String>
+    <system:String x:Key="flowlauncher_plugin_browserbookmark_settings_newTab">Tab mới</system:String>

31-31: Untranslated setting; provide Vietnamese.

Translate and keep “favicon” product term unlocalized.

-    <system:String x:Key="flowlauncher_plugin_browserbookmark_enable_favicons">Load favicons (can be time consuming during startup)</system:String>
+    <system:String x:Key="flowlauncher_plugin_browserbookmark_enable_favicons">Tải favicon (có thể tốn thời gian khi khởi động)</system:String>
Plugins/Flow.Launcher.Plugin.BrowserBookmarks/Languages/es-419.xaml (1)

9-31: Untranslated strings in es-419; propose Spanish localizations and minor copy fixes (URL casing).

Several entries remain in English and “url” should be “URL”. Suggested changes below.

-    <system:String x:Key="flowlauncher_plugin_browserbookmark_copy_failed">Failed to set url in clipboard</system:String>
+    <system:String x:Key="flowlauncher_plugin_browserbookmark_copy_failed">No se pudo copiar la URL al portapapeles</system:String>
@@
-    <system:String x:Key="flowlauncher_plugin_browserbookmark_copyurl_title">Copiar url</system:String>
-    <system:String x:Key="flowlauncher_plugin_browserbookmark_copyurl_subtitle">Copiar la url del marcador al portapapeles</system:String>
+    <system:String x:Key="flowlauncher_plugin_browserbookmark_copyurl_title">Copiar URL</system:String>
+    <system:String x:Key="flowlauncher_plugin_browserbookmark_copyurl_subtitle">Copiar la URL del marcador al portapapeles</system:String>
@@
-    <system:String x:Key="flowlauncher_plugin_browserbookmark_browseBrowserBookmark">Browse</system:String>
-    <system:String x:Key="flowlauncher_plugin_browserbookmark_others">Others</system:String>
-    <system:String x:Key="flowlauncher_plugin_browserbookmark_browserEngine">Browser Engine</system:String>
-    <system:String x:Key="flowlauncher_plugin_browserbookmark_guideMessage01">If you are not using Chrome, Firefox or Edge, or you are using their portable version, you need to add bookmarks data directory and select correct browser engine to make this plugin work.</system:String>
-    <system:String x:Key="flowlauncher_plugin_browserbookmark_guideMessage02">For example: Brave's engine is Chromium; and its default bookmarks data location is: &quot;%LOCALAPPDATA%\BraveSoftware\Brave-Browser\UserData&quot;. For Firefox engine, the bookmarks directory is the userdata folder contains the places.sqlite file.</system:String>
-    <system:String x:Key="flowlauncher_plugin_browserbookmark_enable_favicons">Load favicons (can be time consuming during startup)</system:String>
+    <system:String x:Key="flowlauncher_plugin_browserbookmark_browseBrowserBookmark">Examinar</system:String>
+    <system:String x:Key="flowlauncher_plugin_browserbookmark_others">Otros</system:String>
+    <system:String x:Key="flowlauncher_plugin_browserbookmark_browserEngine">Motor del navegador</system:String>
+    <system:String x:Key="flowlauncher_plugin_browserbookmark_guideMessage01">Si no usas Chrome, Firefox o Edge, o utilizas sus versiones portátiles, debes añadir la carpeta de datos de marcadores y seleccionar el motor de navegador correcto para que este complemento funcione.</system:String>
+    <system:String x:Key="flowlauncher_plugin_browserbookmark_guideMessage02">Por ejemplo: el motor de Brave es Chromium y su ubicación predeterminada de datos de marcadores es: &quot;%LOCALAPPDATA%\BraveSoftware\Brave-Browser\UserData&quot;. Para Firefox, la carpeta de marcadores es la de usuario que contiene el archivo places.sqlite.</system:String>
+    <system:String x:Key="flowlauncher_plugin_browserbookmark_enable_favicons">Cargar favicons (puede tardar durante el inicio)</system:String>
Plugins/Flow.Launcher.Plugin.BrowserBookmarks/Languages/pl.xaml (2)

9-9: English string left in Polish locale.

Provide Polish translation.

Apply:

-    <system:String x:Key="flowlauncher_plugin_browserbookmark_copy_failed">Failed to set url in clipboard</system:String>
+    <system:String x:Key="flowlauncher_plugin_browserbookmark_copy_failed">Nie udało się skopiować adresu URL do schowka</system:String>

18-19: Normalize “URL” capitalization in PL strings.

Minor copy polish for consistency.

-    <system:String x:Key="flowlauncher_plugin_browserbookmark_copyurl_title">Kopiuj adres url</system:String>
-    <system:String x:Key="flowlauncher_plugin_browserbookmark_copyurl_subtitle">Skopiuj adres url zakładki do schowka</system:String>
+    <system:String x:Key="flowlauncher_plugin_browserbookmark_copyurl_title">Kopiuj adres URL</system:String>
+    <system:String x:Key="flowlauncher_plugin_browserbookmark_copyurl_subtitle">Skopiuj adres URL zakładki do schowka</system:String>
Plugins/Flow.Launcher.Plugin.BrowserBookmarks/Languages/ru.xaml (5)

1-1: Normalize UTF‑8 BOM usage across language files

The leading BOM appears intentional. Please confirm all language XAMLs use the same encoding to avoid noisy diffs and potential tooling quirks.


9-9: Untranslated string → provide RU translation

Replace the English fallback with Russian.

Apply this diff:

-    <system:String x:Key="flowlauncher_plugin_browserbookmark_copy_failed">Failed to set url in clipboard</system:String>
+    <system:String x:Key="flowlauncher_plugin_browserbookmark_copy_failed">Не удалось скопировать URL-адрес в буфер обмена</system:String>

31-31: Untranslated string → provide RU translation

Localize the favicon setting.

Apply this diff:

-    <system:String x:Key="flowlauncher_plugin_browserbookmark_enable_favicons">Load favicons (can be time consuming during startup)</system:String>
+    <system:String x:Key="flowlauncher_plugin_browserbookmark_enable_favicons">Загружать значки сайтов (может замедлить запуск)</system:String>

17-17: Wording: use imperative “Выбрать” instead of noun “Выбор”

Improves UI tone consistency with other actions.

Apply this diff:

-    <system:String x:Key="flowlauncher_plugin_browserbookmark_settings_choose">Выбор</system:String>
+    <system:String x:Key="flowlauncher_plugin_browserbookmark_settings_choose">Выбрать</system:String>

22-22: Clarify label to explicitly mention bookmarks

More specific and consistent with other strings.

Apply this diff:

-    <system:String x:Key="flowlauncher_plugin_browserbookmark_browserBookmarkDataDirectory">Путь к каталогу данных</system:String>
+    <system:String x:Key="flowlauncher_plugin_browserbookmark_browserBookmarkDataDirectory">Путь к каталогу данных закладок</system:String>
Plugins/Flow.Launcher.Plugin.BrowserBookmarks/Languages/zh-cn.xaml (5)

9-9: Fix trailing space and terminology (“剪切板” → “剪贴板”).

There’s a trailing space and a common typo. Recommend consistent wording across related strings.

-    <system:String x:Key="flowlauncher_plugin_browserbookmark_copy_failed">无法复制 URL 到剪切板 </system:String>
+    <system:String x:Key="flowlauncher_plugin_browserbookmark_copy_failed">无法将网址复制到剪贴板</system:String>

29-30: Chinese spacing/wording polish in guide messages.

Remove unnecessary spaces around punctuation and consider “便携版” over “绿色版” for zh-CN UI.

-    <system:String x:Key="flowlauncher_plugin_browserbookmark_guideMessage01">如果你没有使用 Chrome、 Firefox 或 Edge,或者正在使用它们的绿色版, 那么你需要添加书签数据目录并选择正确的浏览器引擎才能使此插件正常工作。</system:String>
+    <system:String x:Key="flowlauncher_plugin_browserbookmark_guideMessage01">如果你没有使用 Chrome、Firefox 或 Edge,或者正在使用它们的便携版,那么需要添加书签数据目录并选择正确的浏览器引擎才能使此插件正常工作。</system:String>
-    <system:String x:Key="flowlauncher_plugin_browserbookmark_guideMessage02">例如:Brave 浏览器的引擎是 Chromium;其默认书签数据位置是 &quot;%LOCALAPPDATA%\BraveSoftware\Brave-Browser\UserData&quot;。 对于 Firefox 引擎的浏览器,书签目录是 places.sqlite 文件所在的用户数据文件夹。</system:String>
+    <system:String x:Key="flowlauncher_plugin_browserbookmark_guideMessage02">例如:Brave 浏览器的引擎是 Chromium;其默认书签数据位置是“%LOCALAPPDATA%\BraveSoftware\Brave-Browser\UserData”。对于 Firefox 引擎的浏览器,书签目录是 places.sqlite 文件所在的用户数据文件夹。</system:String>

31-31: “收藏夹图标”/paren style consistency (“网站图标(favicon)”).

Prefer consistent terminology with other locales and full-width parentheses in zh-CN.

-    <system:String x:Key="flowlauncher_plugin_browserbookmark_enable_favicons">载入收藏夹图标 (启动时可能十分耗时)</system:String>
+    <system:String x:Key="flowlauncher_plugin_browserbookmark_enable_favicons">加载网站图标(favicon)(启动时可能较慢)</system:String>

1-3: Optional: use x:String and drop mscorlib assembly hint.

In modern WPF on .NET, x:String is sufficient and avoids mscorlib binding. Not required if consistent with the rest of the repo.

-<ResourceDictionary xmlns="http://schemas.microsoft.com/winfx/2006/xaml/presentation" xmlns:x="http://schemas.microsoft.com/winfx/2006/xaml" xmlns:system="clr-namespace:System;assembly=mscorlib">
+<ResourceDictionary xmlns="http://schemas.microsoft.com/winfx/2006/xaml/presentation" xmlns:x="http://schemas.microsoft.com/winfx/2006/xaml">

And replace system:String with x:String throughout this file.


5-33: Crowdin and settings/cache migration check due to plugin rename.

Since the directory/namespace changed, ensure:

  • Crowdin path mapping is updated so zh-CN continues to sync.
  • Plugin ID/Action keywords migration (if changed) won’t drop user settings/caches without a migration step.

If needed, I can draft a simple migration routine to copy old settings/caches from Flow.Launcher.Plugin.BrowserBookmark to Flow.Launcher.Plugin.BrowserBookmarks.

Plugins/Flow.Launcher.Plugin.BrowserBookmarks/Languages/de.xaml (5)

12-29: Copy may be outdated vs. “automatic engine detection”.

Strings reference manual engine selection; PR states engine detection is automatic. Align the guidance to avoid confusing users, or clarify that manual settings are only for portable/custom installs.

If manual selection is no longer exposed, I can propose updated German copy inline. Want me to draft it?


9-9: German phrasing improvement for error message.

Apply this diff:

-    <system:String x:Key="flowlauncher_plugin_browserbookmark_copy_failed">URL in Zwischenablage konnte nicht festgelegt werden</system:String>
+    <system:String x:Key="flowlauncher_plugin_browserbookmark_copy_failed">URL konnte nicht in die Zwischenablage kopiert werden</system:String>

22-22: Grammar nit.

Apply this diff:

-    <system:String x:Key="flowlauncher_plugin_browserbookmark_browserBookmarkDataDirectory">Pfad zu Datenverzeichnis</system:String>
+    <system:String x:Key="flowlauncher_plugin_browserbookmark_browserBookmarkDataDirectory">Pfad zum Datenverzeichnis</system:String>

32-32: Label may not reflect new async favicon behavior.

If favicons now load in background and not strictly on startup, rephrase to set expectations.

Apply this diff (if accurate):

-    <system:String x:Key="flowlauncher_plugin_browserbookmark_enable_favicons">Favicons laden (kann während des Starts zeitaufwendig sein)</system:String>
+    <system:String x:Key="flowlauncher_plugin_browserbookmark_enable_favicons">Favicons laden (wird im Hintergrund verarbeitet; der erste Durchlauf kann länger dauern)</system:String>

2-2: Crowdin/GitHub Actions path alignment.

The plugin moved to Plugins/Flow.Launcher.Plugin.BrowserBookmarks/. Ensure l10n pipelines (Crowdin config, extraction/upload/download jobs) include this new path, otherwise translations won’t update.

Plugins/Flow.Launcher.Plugin.BrowserBookmarks/Languages/he.xaml (3)

9-9: Translate the remaining English strings for consistency (and RTL rendering).

Two keys are still in English; translate to improve UX and avoid LTR/RTL mixing.

-    <system:String x:Key="flowlauncher_plugin_browserbookmark_copy_failed">Failed to set url in clipboard</system:String>
+    <system:String x:Key="flowlauncher_plugin_browserbookmark_copy_failed">העתקת הכתובת ללוח נכשלה</system:String>

-    <system:String x:Key="flowlauncher_plugin_browserbookmark_enable_favicons">Load favicons (can be time consuming during startup)</system:String>
+    <system:String x:Key="flowlauncher_plugin_browserbookmark_enable_favicons">טעינת סמלי אתרים (עשויה להאט את ההפעלה)</system:String>

Also applies to: 31-31


29-30: Optional RTL polish for mixed-direction text (paths/env vars).

Long Hebrew sentences with Latin paths can render awkwardly. Consider adding LRM around the path/env variable to stabilize direction.

Example:

<system:String x:Key="flowlauncher_plugin_browserbookmark_guideMessage02">
לדוגמה: המנוע של Brave הוא Chromium, ומיקום ברירת המחדל של נתוני הסימניות שלו הוא: &#x200E;%LOCALAPPDATA%\BraveSoftware\Brave-Browser\UserData&#x200E;. עבור מנוע Firefox, ספריית הסימניות היא תיקיית המשתמש שמכילה את הקובץ &#x200E;places.sqlite&#x200E;.
</system:String>

16-17: Remove stale "Set browser from path" / "Choose" localization keys or clarify as manual override

No XAML/C# references found for flowlauncher_plugin_browserbookmark_settings_setBrowserFromPath or flowlauncher_plugin_browserbookmark_settings_choose; engine auto‑detection is implemented (CustomBrowserSettingViewModel.DetectedEngineText uses engine_detection_* and CustomBrowserSetting.xaml displays it). Remove these keys from Plugins/Flow.Launcher.Plugin.BrowserBookmarks/Languages/*.xaml (all locales) or change the copy to explicitly indicate a manual/portable‑browser override (e.g., “Manual path for portable/advanced browsers”).

Plugins/Flow.Launcher.Plugin.BrowserBookmarks/Languages/sr.xaml (3)

9-9: Microcopy/grammar: capitalize “URL” and fix phrasing.

Use consistent capitalization and clearer wording.

Apply:

-    <system:String x:Key="flowlauncher_plugin_browserbookmark_copy_failed">Failed to set url in clipboard</system:String>
+    <system:String x:Key="flowlauncher_plugin_browserbookmark_copy_failed">Failed to copy URL to clipboard</system:String>
-    <system:String x:Key="flowlauncher_plugin_browserbookmark_copyurl_title">Copy url</system:String>
+    <system:String x:Key="flowlauncher_plugin_browserbookmark_copyurl_title">Copy URL</system:String>
-    <system:String x:Key="flowlauncher_plugin_browserbookmark_copyurl_subtitle">Copy the bookmark's url to clipboard</system:String>
+    <system:String x:Key="flowlauncher_plugin_browserbookmark_copyurl_subtitle">Copy the bookmark's URL to clipboard</system:String>

Also applies to: 18-19


31-31: Wording conflicts with async favicon pipeline.

“time consuming during startup” suggests blocking startup; new plugin claims background/parallel favicon work. Adjust to set expectations.

Apply:

-    <system:String x:Key="flowlauncher_plugin_browserbookmark_enable_favicons">Load favicons (can be time consuming during startup)</system:String>
+    <system:String x:Key="flowlauncher_plugin_browserbookmark_enable_favicons">Load favicons (cached in background; initial caching may take time)</system:String>

20-30: Guide messages: minor clarity and locale.

These lengthy English guide strings in sr.xaml likely need translation; also consider breaking into shorter, translatable sentences in en.xaml first.

I can propose concise EN base strings to simplify translation if helpful.

Plugins/Flow.Launcher.Plugin.BrowserBookmarks/Languages/es.xaml (1)

9-9: Spanish typo: “portapeles” → “portapapeles”; also capitalize URL.

Fix improves consistency with Line 19.

-    <system:String x:Key="flowlauncher_plugin_browserbookmark_copy_failed">No se ha podido establecer la url en el portapeles</system:String>
+    <system:String x:Key="flowlauncher_plugin_browserbookmark_copy_failed">No se ha podido establecer la URL en el portapapeles</system:String>
Plugins/Flow.Launcher.Plugin.BrowserBookmarks/Languages/pt-pt.xaml (3)

5-6: Confirm key prefix vs. new plugin ID (possible lookup mismatch).

All keys still use the flowlauncher_plugin_browserbookmark_* prefix, while this PR renames the project to BrowserBookmarks. If the plugin ID or resource lookup prefix changed (e.g., to browserbookmarks), these strings may not be resolved. Please confirm the plugin ID in plugin.json and how resources are looked up. If the old prefix is intentionally kept for continuity, ignore. Otherwise, consider adding temporary alias keys to ease migration.

Option (non-breaking alias if moving to browserbookmarks prefix):

@@
-</ResourceDictionary>
+  <!-- Transitional aliases if the resource lookup expects `browserbookmarks_*` -->
+  <system:String x:Key="flowlauncher_plugin_browserbookmarks_plugin_name">
+    {StaticResource flowlauncher_plugin_browserbookmark_plugin_name}
+  </system:String>
+  <system:String x:Key="flowlauncher_plugin_browserbookmarks_plugin_description">
+    {StaticResource flowlauncher_plugin_browserbookmark_plugin_description}
+  </system:String>
+</ResourceDictionary>

9-9: Portuguese phrasing: “copiar” instead of “definir”.

More natural and consistent with the title “Copiar URL”.

Apply:

-    <system:String x:Key="flowlauncher_plugin_browserbookmark_copy_failed">Falha ao definir o URL na área de transferência</system:String>
+    <system:String x:Key="flowlauncher_plugin_browserbookmark_copy_failed">Falha ao copiar o URL para a área de transferência</system:String>

1-1: BOM/encoding churn across language files.

This file includes a BOM and appears to be a reformat‑only change. To reduce noisy diffs and Crowdin churn, normalize language files to UTF‑8 (consistently with/without BOM) and a single indentation style across all locales.

Would you like a script to scan the Languages folder and report encoding/BOM inconsistencies?

Plugins/Flow.Launcher.Plugin.BrowserBookmarks/Languages/zh-tw.xaml (2)

5-6: Key prefix still “browserbookmark” while project renamed; confirm consistency/migration.

Keys retain the old “browserbookmark” prefix. If plugin ID/namespace changed to BrowserBookmarks, verify:

  • Settings/localization loaders still resolve these keys.
  • Any planned settings migration from the old plugin isn’t blocked by ID or path changes.

If the intent is seamless replacement, consider keeping the old PluginID or add a migration step that copies settings/cache from the previous path. I can draft a migration helper if needed.


1-3: Encoding/BOM and Crowdin path check.

  • File starts with BOM; acceptable for XAML, but ensure your Crowdin config and build tooling expect UTF‑8 with BOM or normalize consistently across language files.
  • The directory path changed to “BrowserBookmarks”; confirm Crowdin mappings and GitHub Actions are updated so translations continue to sync.
Plugins/Flow.Launcher.Plugin.BrowserBookmarks/Languages/ar.xaml (2)

9-9: Untranslated string (“copy failed”).

This should be localized for AR.

Apply:

-    <system:String x:Key="flowlauncher_plugin_browserbookmark_copy_failed">Failed to set url in clipboard</system:String>
+    <system:String x:Key="flowlauncher_plugin_browserbookmark_copy_failed">تعذّر نسخ الرابط إلى الحافظة</system:String>

31-31: Untranslated string (“Load favicons”).

Localize to AR.

-    <system:String x:Key="flowlauncher_plugin_browserbookmark_enable_favicons">Load favicons (can be time consuming during startup)</system:String>
+    <system:String x:Key="flowlauncher_plugin_browserbookmark_enable_favicons">تحميل الأيقونات المفضلة (قد يستغرق وقتاً عند بدء التشغيل)</system:String>
Plugins/Flow.Launcher.Plugin.BrowserBookmarks/Languages/fr.xaml (1)

9-9: French casing/wording nits (“url” → “URL”).

Minor copy polish.

-    <system:String x:Key="flowlauncher_plugin_browserbookmark_copy_failed">Impossible de mettre l'url dans le presse-papiers</system:String>
+    <system:String x:Key="flowlauncher_plugin_browserbookmark_copy_failed">Impossible de copier l’URL dans le presse-papiers</system:String>
-    <system:String x:Key="flowlauncher_plugin_browserbookmark_copyurl_title">Copier l'url</system:String>
+    <system:String x:Key="flowlauncher_plugin_browserbookmark_copyurl_title">Copier l’URL</system:String>
-    <system:String x:Key="flowlauncher_plugin_browserbookmark_copyurl_subtitle">Copier l'Url du favori dans le presse-papier</system:String>
+    <system:String x:Key="flowlauncher_plugin_browserbookmark_copyurl_subtitle">Copier l’URL du favori dans le presse‑papiers</system:String>

Also applies to: 18-19

Plugins/Flow.Launcher.Plugin.BrowserBookmarks/Languages/nl.xaml (1)

5-31: Mixed EN/NL strings — inconsistent localization.

Many entries remain English (name/description, main/guide texts). Either translate here or leave placeholders and rely on Crowdin.

If you want, I can propose NL translations for the unchanged keys.

Plugins/Flow.Launcher.Plugin.BrowserBookmarks/Languages/pt-br.xaml (1)

9-9: Untranslated EN strings (“copy failed”, “Load favicons”).

Localize to PT‑BR.

-    <system:String x:Key="flowlauncher_plugin_browserbookmark_copy_failed">Failed to set url in clipboard</system:String>
+    <system:String x:Key="flowlauncher_plugin_browserbookmark_copy_failed">Falha ao copiar a URL para a área de transferência</system:String>
-    <system:String x:Key="flowlauncher_plugin_browserbookmark_enable_favicons">Load favicons (can be time consuming during startup)</system:String>
+    <system:String x:Key="flowlauncher_plugin_browserbookmark_enable_favicons">Carregar favicons (pode demorar na inicialização)</system:String>

Also applies to: 31-31

Plugins/Flow.Launcher.Plugin.BrowserBookmarks/Languages/ja.xaml (1)

9-9: Untranslated string (“copy failed”).

Localize to JA.

-    <system:String x:Key="flowlauncher_plugin_browserbookmark_copy_failed">Failed to set url in clipboard</system:String>
+    <system:String x:Key="flowlauncher_plugin_browserbookmark_copy_failed">クリップボードにURLをコピーできませんでした</system:String>
Plugins/Flow.Launcher.Plugin.BrowserBookmarks/Services/BookmarkWatcherService.cs (1)

42-51: Consider buffer overflow handling.

Set InternalBufferSize and handle watcher.Error to recover from overflows on heavy writes.

             var watcher = new FileSystemWatcher(directory)
             {
                 Filter = fileName,
-                NotifyFilter = NotifyFilters.LastWrite | NotifyFilters.FileName | NotifyFilters.Size
+                NotifyFilter = NotifyFilters.LastWrite | NotifyFilters.FileName | NotifyFilters.Size,
+                InternalBufferSize = 64 * 1024 // 64KB to reduce overflow risk
             };
+            watcher.Error += (_, __) => TriggerDebouncedReload();
Plugins/Flow.Launcher.Plugin.BrowserBookmarks/Models/Settings.cs (1)

14-48: Guard setters to avoid redundant PropertyChanged churn.

Early‑return when value unchanged. Reduces unnecessary reloads.

-        set { _loadChromeBookmark = value; OnPropertyChanged(); }
+        set { if (_loadChromeBookmark == value) return; _loadChromeBookmark = value; OnPropertyChanged(); }
@@
-        set { _loadFirefoxBookmark = value; OnPropertyChanged(); }
+        set { if (_loadFirefoxBookmark == value) return; _loadFirefoxBookmark = value; OnPropertyChanged(); }
@@
-        set { _loadEdgeBookmark = value; OnPropertyChanged(); }
+        set { if (_loadEdgeBookmark == value) return; _loadEdgeBookmark = value; OnPropertyChanged(); }
@@
-        set { _loadChromiumBookmark = value; OnPropertyChanged(); }
+        set { if (_loadChromiumBookmark == value) return; _loadChromiumBookmark = value; OnPropertyChanged(); }
@@
-        set { _enableFavicons = value; OnPropertyChanged(); }
+        set { if (_enableFavicons == value) return; _enableFavicons = value; OnPropertyChanged(); }
@@
-        set { _fetchMissingFavicons = value; OnPropertyChanged(); }
+        set { if (_fetchMissingFavicons == value) return; _fetchMissingFavicons = value; OnPropertyChanged(); }
Plugins/Flow.Launcher.Plugin.BrowserBookmarks/Languages/it.xaml (1)

1-34: Inconsistent localization - Line 31 remains in English.

While most strings are properly localized to Italian, the favicon loading setting remains in English: "Load favicons (can be time consuming during startup)". This should be translated to Italian for consistency.

-    <system:String x:Key="flowlauncher_plugin_browserbookmark_enable_favicons">Load favicons (can be time consuming during startup)</system:String>
+    <system:String x:Key="flowlauncher_plugin_browserbookmark_enable_favicons">Carica favicons (può richiedere tempo durante l'avvio)</system:String>
Plugins/Flow.Launcher.Plugin.BrowserBookmarks/Services/IBookmarkLoader.cs (1)

9-9: Consider more descriptive property name.

The Name property could be more specific about what it represents (e.g., LoaderName, BrowserName, or SourceName) to avoid confusion with bookmark names.

-    string Name { get; }
+    string LoaderName { get; }
Plugins/Flow.Launcher.Plugin.BrowserBookmarks/Flow.Launcher.Plugin.BrowserBookmarks.csproj (2)

86-99: Duplicate copy of Languages\en.xaml (None + Content)

en.xaml is copied twice (None Update and Content Include). Remove the explicit None entry; the Content wildcard already covers it.

   <ItemGroup>
-    <None Update="plugin.json">
-      <CopyToOutputDirectory>PreserveNewest</CopyToOutputDirectory>
-    </None>
-    <None Update="Languages\en.xaml">
-      <CopyToOutputDirectory>PreserveNewest</CopyToOutputDirectory>
-    </None>
+    <None Update="plugin.json">
+      <CopyToOutputDirectory>PreserveNewest</CopyToOutputDirectory>
+    </None>
     <None Update="Images\bookmark.png">
       <CopyToOutputDirectory>PreserveNewest</CopyToOutputDirectory>
     </None>
     <None Update="Images\copylink.png">
       <CopyToOutputDirectory>PreserveNewest</CopyToOutputDirectory>
     </None>
   </ItemGroup>

Also applies to: 100-105


41-61: Optional: prune non‑Windows runtimes is redundant for netX‑windows TFM

With net9.0‑windows, NuGet should not bring non‑Windows assets. These RemoveDir targets add build time and complexity.

Consider dropping the non‑Windows entries or the entire target unless you’ve measured size wins.

Also applies to: 63-84

Plugins/Flow.Launcher.Plugin.BrowserBookmarks/Services/BrowserDetector.cs (1)

15-25: Harden against IO exceptions and add Chromium “User Data” fallback

Directory enumeration and file checks can throw; also consider when users select the browser root that contains “User Data”.

     public static BrowserType DetectBrowserType(string dataDirectoryPath)
     {
         if (string.IsNullOrEmpty(dataDirectoryPath) || !Directory.Exists(dataDirectoryPath))
             return BrowserType.Unknown;

-        // Check for Chromium-based browsers by looking for the 'Bookmarks' file.
+        // Check for Chromium-based browsers by looking for the 'Bookmarks' file.
         // This includes checking common profile subdirectories.
-        var profileDirectories = Directory.EnumerateDirectories(dataDirectoryPath, "Profile *").ToList();
-        var defaultProfile = Path.Combine(dataDirectoryPath, "Default");
-        if (Directory.Exists(defaultProfile))
-            profileDirectories.Add(defaultProfile);
-        
-        // Also check the root directory itself, as some browsers use it directly.
-        profileDirectories.Add(dataDirectoryPath);
+        var profileDirectories = new List<string>();
+        try
+        {
+            profileDirectories.AddRange(Directory.EnumerateDirectories(dataDirectoryPath, "Profile *"));
+            var defaultProfile = Path.Combine(dataDirectoryPath, "Default");
+            if (Directory.Exists(defaultProfile))
+                profileDirectories.Add(defaultProfile);
+            // Some configs store Bookmarks directly under the data dir.
+            profileDirectories.Add(dataDirectoryPath);
+            // Fallback: typical Chromium root contains a "User Data" dir.
+            var userData = Path.Combine(dataDirectoryPath, "User Data");
+            if (Directory.Exists(userData))
+            {
+                profileDirectories.Add(userData);
+                var userDataDefault = Path.Combine(userData, "Default");
+                if (Directory.Exists(userDataDefault))
+                    profileDirectories.Add(userDataDefault);
+                profileDirectories.AddRange(Directory.EnumerateDirectories(userData, "Profile *"));
+            }
+        }
+        catch
+        {
+            return BrowserType.Unknown;
+        }

         if (profileDirectories.Any(p => File.Exists(Path.Combine(p, "Bookmarks"))))
         {
             return BrowserType.Chromium;
         }

         // Check for Firefox-based browsers by looking for 'places.sqlite'.
         // This leverages the existing FirefoxProfileFinder logic.
-        if (File.Exists(Path.Combine(dataDirectoryPath, "places.sqlite")) || !string.IsNullOrEmpty(FirefoxProfileFinder.GetPlacesPathFromProfileDir(dataDirectoryPath)))
+        try
+        {
+            if (File.Exists(Path.Combine(dataDirectoryPath, "places.sqlite")) ||
+                !string.IsNullOrEmpty(FirefoxProfileFinder.GetPlacesPathFromProfileDir(dataDirectoryPath)))
+            {
+                return BrowserType.Firefox;
+            }
+        }
+        catch
+        {
+            return BrowserType.Unknown;
+        }
-
-        {
-            return BrowserType.Firefox;
-        }

Also applies to: 30-37

Plugins/Flow.Launcher.Plugin.BrowserBookmarks/Views/CustomBrowserSetting.xaml.cs (1)

26-36: Guard Save with CanExecute and mark key events handled

Avoid executing Save when invalid and stop event bubbling.

     private void WindowKeyDown(object sender, KeyEventArgs e)
     {
         if (e.Key == System.Windows.Input.Key.Enter)
         {
-            _viewModel.SaveCommand.Execute(null);
+            if (_viewModel.SaveCommand.CanExecute(null))
+                _viewModel.SaveCommand.Execute(null);
+            e.Handled = true;
         }
         else if (e.Key == System.Windows.Input.Key.Escape)
         {
             _viewModel.CancelCommand.Execute(null);
+            e.Handled = true;
         }
     }
Plugins/Flow.Launcher.Plugin.BrowserBookmarks/Models/CustomBrowser.cs (1)

9-12: Initialize non‑nullable strings

Prevents null states if NRT is enabled and avoids null derefs in bindings.

-    private string _name;
-    private string _dataDirectoryPath;
+    private string _name = string.Empty;
+    private string _dataDirectoryPath = string.Empty;
     private BrowserType _browserType = BrowserType.Unknown;
Plugins/Flow.Launcher.Plugin.BrowserBookmarks/Views/SettingsControl.xaml (1)

27-41: Localize browser labels (Chrome/Chromium/Edge/Firefox)

Hard‑coded strings bypass Crowdin. Use resource keys like other labels.

-            <CheckBox
-                Margin="{StaticResource SettingPanelItemRightMargin}"
-                Content="Chrome"
-                IsChecked="{Binding Settings.LoadChromeBookmark}" />
+            <CheckBox
+                Margin="{StaticResource SettingPanelItemRightMargin}"
+                Content="{DynamicResource flowlauncher_plugin_browserbookmark_browser_chrome}"
+                IsChecked="{Binding Settings.LoadChromeBookmark}" />
@@
-            <CheckBox
-                Margin="{StaticResource SettingPanelItemRightMargin}"
-                Content="Chromium"
-                IsChecked="{Binding Settings.LoadChromiumBookmark}" />
+            <CheckBox
+                Margin="{StaticResource SettingPanelItemRightMargin}"
+                Content="{DynamicResource flowlauncher_plugin_browserbookmark_browser_chromium}"
+                IsChecked="{Binding Settings.LoadChromiumBookmark}" />
@@
-            <CheckBox
-                Margin="{StaticResource SettingPanelItemRightMargin}"
-                Content="Edge"
-                IsChecked="{Binding Settings.LoadEdgeBookmark}" />
+            <CheckBox
+                Margin="{StaticResource SettingPanelItemRightMargin}"
+                Content="{DynamicResource flowlauncher_plugin_browserbookmark_browser_edge}"
+                IsChecked="{Binding Settings.LoadEdgeBookmark}" />
@@
-            <CheckBox
-                Margin="{StaticResource SettingPanelItemRightMargin}"
-                Content="Firefox"
-                IsChecked="{Binding Settings.LoadFirefoxBookmark}" />
+            <CheckBox
+                Margin="{StaticResource SettingPanelItemRightMargin}"
+                Content="{DynamicResource flowlauncher_plugin_browserbookmark_browser_firefox}"
+                IsChecked="{Binding Settings.LoadFirefoxBookmark}" />

I can add the en.xaml entries if you want.

Plugins/Flow.Launcher.Plugin.BrowserBookmarks/Views/SettingsControl.xaml.cs (1)

33-39: Use bound SelectedCustomBrowser consistently

Delete uses the element’s SelectedItem while Edit uses the bound property. Use one pattern for clarity.

-        if (CustomBrowsers.SelectedItem is CustomBrowser selectedCustomBrowser)
-        {
-            Settings.CustomBrowsers.Remove(selectedCustomBrowser);
-        }
+        if (SelectedCustomBrowser is not null)
+            Settings.CustomBrowsers.Remove(SelectedCustomBrowser);
Plugins/Flow.Launcher.Plugin.BrowserBookmarks/Services/FirefoxBookmarkLoader.cs (1)

39-83: Consider adding defensive protection against very large bookmark collections.

The current implementation loads all bookmarks into memory before yielding them. For users with extensive bookmark collections, this could cause memory pressure. Consider yielding bookmarks directly as they're read from the database instead of accumulating them first.

Apply this diff to stream bookmarks directly:

 public async IAsyncEnumerable<Bookmark> GetBookmarksAsync([EnumeratorCancellation] CancellationToken cancellationToken = default)
 {
     if (string.IsNullOrEmpty(_placesPath) || !File.Exists(_placesPath))
         yield break;

-    var bookmarks = new List<Bookmark>();
     string? tempDbPath = null;

     try
     {
         // First, try to read directly from the source to avoid a slow file copy
-        await ReadBookmarksFromDb(_placesPath, bookmarks, cancellationToken);
+        await foreach (var bookmark in ReadBookmarksFromDbAsync(_placesPath, cancellationToken))
+        {
+            yield return bookmark;
+        }
     }
     catch (SqliteException ex) when (ex.SqliteErrorCode is 5 or 6) // 5 is SQLITE_BUSY, 6 is SQLITE_LOCKED
     {
         // Fallback to copying the file if the database is locked (e.g., Firefox is open)
         try
         {
             tempDbPath = Path.Combine(_tempPath, $"ff_places_{Guid.NewGuid()}.sqlite");
             File.Copy(_placesPath, tempDbPath, true);
-            await ReadBookmarksFromDb(tempDbPath, bookmarks, cancellationToken);
+            await foreach (var bookmark in ReadBookmarksFromDbAsync(tempDbPath, cancellationToken))
+            {
+                yield return bookmark;
+            }
         }
         catch (Exception copyEx)
         {
             _logException(nameof(FirefoxBookmarkLoader), $"Failed to load {_browserName} bookmarks from fallback copy: {_placesPath}", copyEx);
         }
     }
     catch (Exception e)
     {
         _logException(nameof(FirefoxBookmarkLoader), $"Failed to load {_browserName} bookmarks: {_placesPath}", e);
     }
     finally
     {
         if (tempDbPath != null && File.Exists(tempDbPath))
         {
             try { File.Delete(tempDbPath); } 
             catch(Exception e) { _logException(nameof(FirefoxBookmarkLoader), $"Failed to delete temp db file {tempDbPath}", e); }
         }
     }
-
-    foreach (var bookmark in bookmarks)
-    {
-        yield return bookmark;
-    }
 }

And update the helper method:

-private async Task ReadBookmarksFromDb(string dbPath, ICollection<Bookmark> bookmarks, CancellationToken cancellationToken)
+private async IAsyncEnumerable<Bookmark> ReadBookmarksFromDbAsync(string dbPath, [EnumeratorCancellation] CancellationToken cancellationToken = default)
 {
     var profilePath = Path.GetDirectoryName(dbPath) ?? string.Empty;
     var connectionString = $"Data Source={dbPath};Mode=ReadOnly;Pooling=false;";
     
     await using var dbConnection = new SqliteConnection(connectionString);
     await dbConnection.OpenAsync(cancellationToken);
     await using var command = new SqliteCommand(QueryAllBookmarks, dbConnection);
     await using var reader = await command.ExecuteReaderAsync(cancellationToken);

     while (await reader.ReadAsync(cancellationToken))
     {
         var title = reader["title"]?.ToString() ?? string.Empty;
         var url = reader["url"]?.ToString();

         if (!string.IsNullOrEmpty(url))
         {
-            bookmarks.Add(new Bookmark(title, url, _browserName, profilePath));
+            yield return new Bookmark(title, url, _browserName, profilePath);
         }
     }
 }
Plugins/Flow.Launcher.Plugin.BrowserBookmarks/Services/LocalFaviconExtractor.cs (2)

62-90: Consider implementing retry logic for locked database scenarios.

When copying the database file fails due to it being locked or in use, consider implementing retry logic with exponential backoff. This would improve reliability, especially during browser updates or when antivirus software temporarily locks files.

Example retry implementation:

+private async Task<string?> CopyDatabaseWithRetryAsync(string sourcePath, string destPath, int maxRetries = 3, CancellationToken token = default)
+{
+    for (int i = 0; i < maxRetries; i++)
+    {
+        try
+        {
+            File.Copy(sourcePath, destPath, true);
+            return destPath;
+        }
+        catch (IOException) when (i < maxRetries - 1)
+        {
+            await Task.Delay(TimeSpan.FromMilliseconds(100 * Math.Pow(2, i)), token);
+        }
+    }
+    return null;
+}

 try
 {
-    File.Copy(dbPath, tempDbPath, true);
+    var copiedPath = await CopyDatabaseWithRetryAsync(dbPath, tempDbPath, 3, token);
+    if (copiedPath == null)
+    {
+        _context.API.LogDebug(nameof(LocalFaviconExtractor), $"Failed to copy database after retries: {dbPath}");
+        return null;
+    }

27-28: Use BrowserType enum instead of string.Contains on Bookmark.Source

LocalFaviconExtractor.GetFaviconDataAsync currently checks bookmark.Source.Contains("Firefox") — brittle. Use the existing BrowserType (Models/CustomBrowser.cs): either add a BrowserType property to Bookmark and set it in the loaders, or resolve bookmark.Source to the matching CustomBrowser and use its BrowserType to choose Firefox vs Chromium DB/query.

Plugins/Flow.Launcher.Plugin.BrowserBookmarks/Services/BookmarkLoaderService.cs (1)

104-131: Exception handling for FirefoxProfileFinder calls is redundant.

The try-catch blocks around FirefoxProfileFinder.GetFirefoxPlacesPath() and GetFirefoxMsixPlacesPath() are unnecessary since these methods already have internal exception handling and return null on failure. The current approach logs exceptions that will never be thrown.

Apply this diff to remove redundant exception handling:

 if (_settings.LoadFirefoxBookmark)
 {
-    string? placesPath = null;
-    try
-    {
-        placesPath = FirefoxProfileFinder.GetFirefoxPlacesPath();
-    }
-    catch (Exception ex)
-    {
-        _context.API.LogException(nameof(BookmarkLoaderService), "Failed to find Firefox profile", ex);
-    }
+    var placesPath = FirefoxProfileFinder.GetFirefoxPlacesPath();
     if (!string.IsNullOrEmpty(placesPath))
     {
         yield return new FirefoxBookmarkLoader("Firefox", placesPath, _tempPath, logAction);
     }

-    string? msixPlacesPath = null;
-    try
-    {
-        msixPlacesPath = FirefoxProfileFinder.GetFirefoxMsixPlacesPath();
-    }
-    catch (Exception ex)
-    {
-        _context.API.LogException(nameof(BookmarkLoaderService), "Failed to find Firefox MSIX package", ex);
-    }
+    var msixPlacesPath = FirefoxProfileFinder.GetFirefoxMsixPlacesPath();
     if (!string.IsNullOrEmpty(msixPlacesPath))
     {
         yield return new FirefoxBookmarkLoader("Firefox (Store)", msixPlacesPath, _tempPath, logAction);
     }
 }
Plugins/Flow.Launcher.Plugin.BrowserBookmarks/Services/FaviconService.cs (4)

248-257: Avoid O(n²) StringBuilder.ToString() in the read loop.

Converting the entire buffer to a string on each iteration scales poorly. Check only the tail segment.

Apply this diff:

-                if (contentBuilder.ToString().Contains("</head>", StringComparison.OrdinalIgnoreCase))
+                var tailStart = Math.Max(0, contentBuilder.Length - 64);
+                if (contentBuilder
+                    .ToString(tailStart, contentBuilder.Length - tailStart)
+                    .IndexOf("</head>", StringComparison.OrdinalIgnoreCase) >= 0)
                 {
                     break;
                 }

348-353: Preserve SVG aspect ratio when scaling to 32x32.

Current independent X/Y scaling can distort icons. Use uniform scale and center.

Apply this diff:

-                using var bitmap = new SKBitmap(32, 32);
-                using var canvas = new SKCanvas(bitmap);
-                canvas.Clear(SKColors.Transparent);
-                var scaleMatrix = SKMatrix.CreateScale(32 / svg.Picture.CullRect.Width, 32 / svg.Picture.CullRect.Height);
-                canvas.DrawPicture(svg.Picture, in scaleMatrix);
+                using var bitmap = new SKBitmap(32, 32);
+                using var canvas = new SKCanvas(bitmap);
+                canvas.Clear(SKColors.Transparent);
+                var rect = svg.Picture.CullRect;
+                var scale = Math.Min(32f / rect.Width, 32f / rect.Height);
+                var tx = (32f - rect.Width * scale) / 2f;
+                var ty = (32f - rect.Height * scale) / 2f;
+                var m = SKMatrix.CreateScale(scale, scale);
+                m = m.PostConcat(SKMatrix.CreateTranslation(tx, ty));
+                canvas.DrawPicture(svg.Picture, in m);

364-387: WPF imaging on background threads may require STA.

IconBitmapDecoder/PngBitmapEncoder (WPF) can throw in MTA. Consider a non‑WPF ICO parser (or ImageSharp) to avoid apartment issues, or marshal this block to an STA.

Would you like a minimal ICO reader replacement using ImageSharp or a small ICO parser?


111-120: Optional: prefer SHA‑256 for cache keys.

SHA‑1 is fine for non‑security use, but SHA‑256 avoids collision debates at negligible cost.

Apply this diff:

-        var hash = SHA1.HashData(Encoding.UTF8.GetBytes(url));
+        var hash = SHA256.HashData(Encoding.UTF8.GetBytes(url));

(Remember to add using System.Security.Cryptography; if not already present.)

Plugins/Flow.Launcher.Plugin.BrowserBookmarks/Main.cs (3)

73-75: Use Volatile.Read to pair with the Volatile.Write.

Ensures readers observe a fully‑published list version.

Apply this diff:

-        var bookmarks = _bookmarks; // use a local copy
+        var bookmarks = Volatile.Read(ref _bookmarks); // read with a memory barrier

40-50: Add a one‑time migration to preserve old plugin caches/settings after rename.

Renaming the plugin directory/namespace risks users losing settings and favicon cache. Migrate the old cache on first run.

Apply this diff and add the helper below:

         var tempPath = SetupTempDirectory();
         
         _bookmarkLoader = new BookmarkLoaderService(Context, _settings, tempPath);
         _faviconService = new FaviconService(Context, _settings, tempPath);
+        TryMigrateOldCachesAndSettings();

Add this helper (outside the shown range):

// Place inside Main class
private void TryMigrateOldCachesAndSettings()
{
    try
    {
        // Adjust old IDs/paths if different; this assumes prior plugin folder name
        var oldCache = Path.Combine(Context.CurrentPluginMetadata.PluginDirectory, "..", "Flow.Launcher.Plugin.BrowserBookmark", "Cache", "FaviconCache");
        var newCache = Path.Combine(Context.CurrentPluginMetadata.PluginCacheDirectoryPath, "FaviconCache");
        if (Directory.Exists(oldCache) && !Directory.Exists(newCache))
        {
            Directory.CreateDirectory(Path.GetDirectoryName(newCache)!);
            Directory.Move(oldCache, newCache);
            Context.API.LogInfo(nameof(Main), "Migrated old favicon cache.");
        }
        // If settings storage key changed, consider reading the old JSON and copying relevant fields.
    }
    catch (Exception ex)
    {
        Context.API.LogException(nameof(Main), "Cache/settings migration failed.", ex);
    }
}

152-159: Fire‑and‑forget timer loop: capture exceptions to avoid unobserved faults.

Wrap the loop in a try/catch to ensure background exceptions are logged.

Apply this diff:

-        _ = Task.Run(async () =>
-        {
-            while (await _firefoxBookmarkTimer.WaitForNextTickAsync(_cancellationTokenSource.Token))
-            {
-                await ReloadFirefoxBookmarksAsync();
-            }
-        }, _cancellationTokenSource.Token);
+        _ = Task.Run(async () =>
+        {
+            try
+            {
+                while (await _firefoxBookmarkTimer.WaitForNextTickAsync(_cancellationTokenSource.Token))
+                    await ReloadFirefoxBookmarksAsync();
+            }
+            catch (OperationCanceledException) { }
+            catch (Exception ex)
+            {
+                Context.API.LogException(nameof(Main), "Firefox periodic reload loop crashed.", ex);
+            }
+        }, _cancellationTokenSource.Token);
Plugins/Flow.Launcher.Plugin.BrowserBookmarks/Views/CustomBrowserSetting.xaml (1)

14-14: MVVM: remove code‑behind KeyDown and bind keys to commands.

Avoid the WindowKeyDown handler; wire Enter/Escape to Save/Cancel.

Apply this diff:

-    KeyDown="WindowKeyDown"

Add key bindings:

   mc:Ignorable="d">
+    <Window.InputBindings>
+        <KeyBinding Key="Enter" Command="{Binding SaveCommand}" />
+        <KeyBinding Key="Escape" Command="{Binding CancelCommand}" />
+    </Window.InputBindings>
Flow.Launcher.sln (1)

72-73: Project type GUID differs from other C# SDK projects.

Most projects use {9A19103F-…} (SDK‑style). If this plugin is SDK‑style too, consider aligning for consistency.

Plugins/Flow.Launcher.Plugin.BrowserBookmarks/Languages/en.xaml (1)

8-8: Clarify description: favicons can come from local DBs too.

Minor copy tweak to reflect behavior accurately.

Apply this diff:

-    <system:String x:Key="flowlauncher_plugin_browserbookmark_plugin_description">Search your browser bookmarks, retrieve favicons from the web.</system:String>
+    <system:String x:Key="flowlauncher_plugin_browserbookmark_plugin_description">Search your browser bookmarks; show favicons (local cache + optional web fetch).</system:String>

Comment on lines +9 to +10
<RootNamespace>Flow.Launcher.Plugin.BrowserBookmarks</RootNamespace>
<AssemblyName>Flow.Launcher.Plugin.BrowserBookmarks</AssemblyName>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

🧩 Analysis chain

Namespace/assembly rename will reset user settings and caches without a migration

Renaming the project/folder changes the plugin path; unless plugin.json keeps the same ID or you migrate on first run, users lose settings, custom browsers, and favicon cache.

  • Prefer keeping the old plugin ID in the new plugin.json to preserve settings.
  • Or ship a one‑time migration in Main/Init: if old settings exist and new are default, copy settings + favicon cache dir.

Example migration sketch (outside this file):

// On plugin init
var oldId = "BROWSERBOOKMARK"; // previous plugin ID
var newId = context.Metadata.ID; // current
if (!string.Equals(oldId, newId, StringComparison.OrdinalIgnoreCase))
{
    var oldData = StorageHelper.GetPluginDataDirectory(oldId);
    var newData = StorageHelper.GetPluginDataDirectory(newId);

    if (Directory.Exists(oldData) && !Directory.Exists(newData))
    {
        Directory.CreateDirectory(newData);
        foreach (var dir in new[] { "Cache", "Favicons" })
        {
            var src = Path.Combine(oldData, dir);
            var dst = Path.Combine(newData, dir);
            if (Directory.Exists(src) && !Directory.Exists(dst))
                DirectoryCopy(src, dst, true);
        }
        // Migrate settings if applicable
        SettingsMigration.TryMigrate(oldId, newId);
    }
}

Run to verify ID alignment and Crowdin/workflow references:

Also applies to: 22-23, 32-33


🏁 Script executed:

#!/bin/bash
# plugin IDs and names
fd -a -t f "plugin.json" -E "bin" -E "obj" | while read f; do
  echo "--- $f"
  rg -nP '"(ID|Name)"\s*:\s*".*"' "$f"
done

# check workflows/crowdin paths mention old/new dirs
rg -nP 'Flow\.Launcher\.Plugin\.BrowserBookmark|Flow\.Launcher\.Plugin\.BrowserBookmarks' .github/ -S || true
rg -nP 'Plugins/Flow\.Launcher\.Plugin\.BrowserBookmark|Plugins/Flow\.Launcher\.Plugin\.BrowserBookmarks' crowdin* .github/ -S || true

Length of output: 2964


Preserve plugin ID and fix CI/workflows after rename

Plugins/Flow.Launcher.Plugin.BrowserBookmarks/plugin.json contains ID "0ECADE17459B49F587BF81DC3A125110"; .github/workflows/default_plugins.yml still references 'Plugins/Flow.Launcher.Plugin.BrowserBookmark' and publishes Flow.Launcher.Plugin.BrowserBookmark.csproj (lines ~40,45–53). This mismatch will break CI and can reset user settings/caches if the plugin ID or assembly name changed.

  • Keep the historical plugin ID in plugin.json (preferred) OR implement a one‑time migration on plugin init (Main/Init) to copy StorageHelper data directory (Cache, Favicons) and migrate settings from the old ID to the new ID.
  • Update .github/workflows/default_plugins.yml (lines ~40,45–53) and any crowdin/workflow references to the new folder/project name, or revert the project/assembly rename so packaging/CI paths match.
🤖 Prompt for AI Agents
In
Plugins/Flow.Launcher.Plugin.BrowserBookmarks/Flow.Launcher.Plugin.BrowserBookmarks.csproj
around lines 9-10, the AssemblyName/RootNamespace rename has caused a mismatch
with plugin.json ID and .github/workflows/default_plugins.yml references (lines
~40,45–53), which will break CI and may reset user data; fix it by either
restoring the original plugin ID in plugin.json (preferred) or implementing a
one-time migration in the plugin Init (copy StorageHelper data directories —
Cache, Favicons — and migrate settings from the old ID to the new ID), and
update .github/workflows/default_plugins.yml and any crowdin/workflow references
to point to the new project/folder name (or revert the project/assembly rename)
so CI packaging paths and plugin ID remain consistent.

Comment on lines 59 to 61
$(OutputPath)runtimes\win-arm;
$(OutputPath)runtimes\win-arm64;"/>
$(OutputPath)runtimes\win-arm64;" />
</Target>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Don’t delete win‑arm64 runtime assets — breaks native ARM64 on Windows

Removing win-arm64 will likely break Skia/Svg native loading on ARM64 Flow builds. Keep ARM64; prune only non‑Windows if needed.

Apply this diff to both targets:

-                            $(OutputPath)runtimes\win-arm;
-                            $(OutputPath)runtimes\win-arm64;" />
+                            $(OutputPath)runtimes\win-arm;" />
-                            $(PublishDir)runtimes\win-arm;
-                            $(PublishDir)runtimes\win-arm64;" />
+                            $(PublishDir)runtimes\win-arm;" />

Also applies to: 83-84

🤖 Prompt for AI Agents
Plugins/Flow.Launcher.Plugin.BrowserBookmarks/Flow.Launcher.Plugin.BrowserBookmarks.csproj
lines ~59-61 (and likewise lines ~83-84): the runtimes list removed "win-arm64"
which will break native ARM64 (Skia/Svg) on Windows; restore "win-arm64" into
the OutputPath runtimes entries for both targets (do not remove win-arm either),
and if pruning is required restrict removals to non-Windows runtimes only so
Windows ARM/ARM64 assets remain packaged.

Comment on lines +28 to +30
<system:String x:Key="flowlauncher_plugin_browserbookmark_browserEngine">Jádro webového prohlížeče</system:String>
<system:String x:Key="flowlauncher_plugin_browserbookmark_guideMessage01">Pokud nepoužíváte prohlížeč Chrome, Firefox nebo Edge nebo používáte přenosnou verzi prohlížeče Chrome, Firefox nebo Edge, musíte přidat složku záložek a vybrat správné jádro prohlížeče, aby tento doplněk fungoval.</system:String>
<system:String x:Key="flowlauncher_plugin_browserbookmark_guideMessage02">Například: prohlížeč Brave má jádro Chromium; výchozí umístění pro data záložek je: &quot;%LOCALAPPDATA%\BraveSoftware\Brave-Browser\UserData&quot;. V případě jádra Firefox je složkou záložek složka userdata, která obsahuje soubor places.sqlite.</system:String>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Text contradicts “automatic engine detection” claim.

Strings instruct users to select a browser engine, but PR states engine detection is automatic. This can mislead users.

Option A (prefer): Update copy to reflect auto‑detection and limit manual steps to portable/custom paths.

-    <system:String x:Key="flowlauncher_plugin_browserbookmark_browserEngine">Jádro webového prohlížeče</system:String>
-    <system:String x:Key="flowlauncher_plugin_browserbookmark_guideMessage01">Pokud nepoužíváte prohlížeč Chrome, Firefox nebo Edge nebo používáte přenosnou verzi prohlížeče Chrome, Firefox nebo Edge, musíte přidat složku záložek a vybrat správné jádro prohlížeče, aby tento doplněk fungoval.</system:String>
+    <system:String x:Key="flowlauncher_plugin_browserbookmark_browserEngine">Jádro prohlížeče (detekováno automaticky)</system:String>
+    <system:String x:Key="flowlauncher_plugin_browserbookmark_guideMessage01">Jádro prohlížeče se obvykle detekuje automaticky. U portable nebo nestandardních instalací přidejte složku záložek ručně.</system:String>

Option B: If engine selection is removed from UI, delete these strings and purge references.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
<system:String x:Key="flowlauncher_plugin_browserbookmark_browserEngine">Jádro webového prohlížeče</system:String>
<system:String x:Key="flowlauncher_plugin_browserbookmark_guideMessage01">Pokud nepoužíváte prohlížeč Chrome, Firefox nebo Edge nebo používáte přenosnou verzi prohlížeče Chrome, Firefox nebo Edge, musíte přidat složku záložek a vybrat správné jádro prohlížeče, aby tento doplněk fungoval.</system:String>
<system:String x:Key="flowlauncher_plugin_browserbookmark_guideMessage02">Například: prohlížeč Brave má jádro Chromium; výchozí umístění pro data záložek je: &quot;%LOCALAPPDATA%\BraveSoftware\Brave-Browser\UserData&quot;. V případě jádra Firefox je složkou záložek složka userdata, která obsahuje soubor places.sqlite.</system:String>
<system:String x:Key="flowlauncher_plugin_browserbookmark_browserEngine">Jádro prohlížeče (detekováno automaticky)</system:String>
<system:String x:Key="flowlauncher_plugin_browserbookmark_guideMessage01">Jádro prohlížeče se obvykle detekuje automaticky. U portable nebo nestandardních instalací přidejte složku záložek ručně.</system:String>
<system:String x:Key="flowlauncher_plugin_browserbookmark_guideMessage02">Například: prohlížeč Brave má jádro Chromium; výchozí umístění pro data záložek je: &quot;%LOCALAPPDATA%\BraveSoftware\Brave-Browser\UserData&quot;. V případě jádra Firefox je složkou záložek složka userdata, která obsahuje soubor places.sqlite.</system:String>
🤖 Prompt for AI Agents
In Plugins/Flow.Launcher.Plugin.BrowserBookmarks/Languages/cs.xaml around lines
28-30, the guide strings instruct users to select a browser engine which
contradicts the plugin's automatic engine detection; update the Czech copy to
state that the plugin detects the browser engine automatically and only requires
manual selection/path input for portable or non-standard installs (or, if the UI
no longer exposes engine selection, remove these strings and any code references
to them). Ensure the revised text mentions automatic detection first, gives a
brief note about portable/custom paths and example paths (Brave/Firefox) if
necessary, and keep keys unchanged so localization lookups remain valid.

Comment on lines +30 to +31
<system:String x:Key="flowlauncher_plugin_browserbookmark_guideMessage02">Zum Beispiel: Die Engine von Brave ist Chromium, und deren Standardspeicherort der Lesezeichen-Daten ist:
%LOCALAPPDATA%\BraveSoftware\Brave-Browser\UserData&quot;. Bei der Firefox-Engine ist das Lesezeichenverzeichnis der Ordner userdata, der die Datei places.sqlite enthält.</system:String>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Fix stray entity and incorrect/dated path; keep value on one line.

There’s a visible “"” artifact and the Brave path should be “User Data” (with space). Also, the line break inside the value is unnecessary.

Apply this diff:

-    <system:String x:Key="flowlauncher_plugin_browserbookmark_guideMessage02">Zum Beispiel: Die Engine von Brave ist Chromium, und deren Standardspeicherort der Lesezeichen-Daten ist:
-%LOCALAPPDATA%\BraveSoftware\Brave-Browser\UserData&quot;. Bei der Firefox-Engine ist das Lesezeichenverzeichnis der Ordner userdata, der die Datei places.sqlite enthält.</system:String>
+    <system:String x:Key="flowlauncher_plugin_browserbookmark_guideMessage02">Zum Beispiel: Brave basiert auf Chromium; der Standardspeicherort der Lesezeichendaten ist %LOCALAPPDATA%\BraveSoftware\Brave-Browser\User Data. Bei Firefox befindet sich das Lesezeichenverzeichnis im Profilordner, der die Datei places.sqlite enthält.</system:String>
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
<system:String x:Key="flowlauncher_plugin_browserbookmark_guideMessage02">Zum Beispiel: Die Engine von Brave ist Chromium, und deren Standardspeicherort der Lesezeichen-Daten ist:
%LOCALAPPDATA%\BraveSoftware\Brave-Browser\UserData&quot;. Bei der Firefox-Engine ist das Lesezeichenverzeichnis der Ordner userdata, der die Datei places.sqlite enthält.</system:String>
<system:String x:Key="flowlauncher_plugin_browserbookmark_guideMessage02">Zum Beispiel: Brave basiert auf Chromium; der Standardspeicherort der Lesezeichendaten ist %LOCALAPPDATA%\BraveSoftware\Brave-Browser\User Data. Bei Firefox befindet sich das Lesezeichenverzeichnis im Profilordner, der die Datei places.sqlite enthält.</system:String>
🤖 Prompt for AI Agents
In Plugins/Flow.Launcher.Plugin.BrowserBookmarks/Languages/de.xaml around lines
30 to 31, the string value contains a stray HTML entity (&quot;), an incorrect
Brave bookmarks path and an unwanted newline; replace the value so it is a
single line, remove the &quot; entity, and correct the Brave path to
"%LOCALAPPDATA%\BraveSoftware\Brave-Browser\User Data" (note the space) while
keeping the rest of the sentence intact.

Comment on lines +2 to +33
<ResourceDictionary xmlns="http://schemas.microsoft.com/winfx/2006/xaml/presentation" xmlns:x="http://schemas.microsoft.com/winfx/2006/xaml" xmlns:system="clr-namespace:System;assembly=mscorlib">

<!-- Plugin Info -->
<system:String x:Key="flowlauncher_plugin_browserbookmark_plugin_name">Marcadores del navegador</system:String>
<system:String x:Key="flowlauncher_plugin_browserbookmark_plugin_description">Busca en los marcadores del navegador</system:String>

<!-- Main -->
<system:String x:Key="flowlauncher_plugin_browserbookmark_copy_failed">No se ha podido establecer la url en el portapeles</system:String>

<!-- Settings -->
<system:String x:Key="flowlauncher_plugin_browserbookmark_bookmarkDataSetting">Datos del marcador</system:String>
<system:String x:Key="flowlauncher_plugin_browserbookmark_settings_openBookmarks">Abrir marcadores en:</system:String>
<system:String x:Key="flowlauncher_plugin_browserbookmark_settings_newWindow">Nueva ventana</system:String>
<system:String x:Key="flowlauncher_plugin_browserbookmark_settings_newTab">Nueva pestaña</system:String>
<system:String x:Key="flowlauncher_plugin_browserbookmark_settings_setBrowserFromPath">Establecer navegador desde ruta:</system:String>
<system:String x:Key="flowlauncher_plugin_browserbookmark_settings_choose">Elegir</system:String>
<system:String x:Key="flowlauncher_plugin_browserbookmark_copyurl_title">Copiar url</system:String>
<system:String x:Key="flowlauncher_plugin_browserbookmark_copyurl_subtitle">Copia la url del marcador al portapapeles</system:String>
<system:String x:Key="flowlauncher_plugin_browserbookmark_loadBrowserFrom">Cargar navegador desde:</system:String>
<system:String x:Key="flowlauncher_plugin_browserbookmark_browserName">Nombre del navegador</system:String>
<system:String x:Key="flowlauncher_plugin_browserbookmark_browserBookmarkDataDirectory">Ruta del directorio de datos</system:String>
<system:String x:Key="flowlauncher_plugin_browserbookmark_addBrowserBookmark">Añadir</system:String>
<system:String x:Key="flowlauncher_plugin_browserbookmark_editBrowserBookmark">Editar</system:String>
<system:String x:Key="flowlauncher_plugin_browserbookmark_removeBrowserBookmark">Eliminar</system:String>
<system:String x:Key="flowlauncher_plugin_browserbookmark_browseBrowserBookmark">Navegar</system:String>
<system:String x:Key="flowlauncher_plugin_browserbookmark_others">Otros</system:String>
<system:String x:Key="flowlauncher_plugin_browserbookmark_browserEngine">Motor del navegador</system:String>
<system:String x:Key="flowlauncher_plugin_browserbookmark_guideMessage01">Si no está utilizando Chrome, Firefox o Edge, o si está utilizando su versión portable, debe añadir el directorio de datos de los marcadores y seleccionar el motor del navegador correcto para que este complemento funcione.</system:String>
<system:String x:Key="flowlauncher_plugin_browserbookmark_guideMessage02">Por ejemplo: El motor de Brave es Chromium; y la ubicación por defecto de los datos de los marcadores es: &quot;%LOCALAPPDATA%\BraveSoftware\Brave-Browser\UserData&quot;. Para el motor de Firefox, el directorio de los marcadores es la carpeta de datos del usuario que contiene el archivo places.sqlite.</system:String>
<system:String x:Key="flowlauncher_plugin_browserbookmark_enable_favicons">Cargar favicons (puede llevar mucho tiempo durante el arranque)</system:String>

</ResourceDictionary>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

🧩 Analysis chain

Ensure parity with en.xaml (new strings likely missing).

Given the new features (favicon fetching, watcher options, auto-detection), Spanish likely lacks new keys added in English. Sync to avoid fallback.

Generate a missing-keys report:

I can help draft Spanish strings once you confirm the missing keys.


🏁 Script executed:

#!/bin/bash
set -euo pipefail
# Extract and diff keys ignoring prefix variants
rg -nPo 'x:Key="\K[^"]+' -- Plugins/Flow.Launcher.Plugin.BrowserBookmarks/Languages/en.xaml | sort > /tmp/en.keys
rg -nPo 'x:Key="\K[^"]+' -- Plugins/Flow.Launcher.Plugin.BrowserBookmarks/Languages/es.xaml | sort > /tmp/es.keys
echo "Keys in EN but not in ES:"
comm -23 /tmp/en.keys /tmp/es.keys || true

Length of output: 1247


Add missing Spanish translation keys to es.xaml

EN keys present in Plugins/Flow.Launcher.Plugin.BrowserBookmarks/Languages/en.xaml but missing from Plugins/Flow.Launcher.Plugin.BrowserBookmarks/Languages/es.xaml — add Spanish translations for:

  • flowlauncher_plugin_browserbookmark_plugin_name
  • flowlauncher_plugin_browserbookmark_plugin_description
  • flowlauncher_plugin_browserbookmark_copy_failed
  • flowlauncher_plugin_browserbookmark_copyurl_title
  • flowlauncher_plugin_browserbookmark_copyurl_subtitle
  • flowlauncher_plugin_browserbookmark_loadBrowserFrom
  • flowlauncher_plugin_browserbookmark_others
  • flowlauncher_plugin_browserbookmark_addBrowserBookmark
  • flowlauncher_plugin_browserbookmark_editBrowserBookmark
  • flowlauncher_plugin_browserbookmark_removeBrowserBookmark
  • flowlauncher_plugin_browserbookmark_enable_favicons
  • flowlauncher_plugin_browserbookmark_fetch_missing_favicons
  • flowlauncher_plugin_browserbookmark_bookmarkDataSetting
  • flowlauncher_plugin_browserbookmark_browserName
  • flowlauncher_plugin_browserbookmark_browserBookmarkDataDirectory
  • flowlauncher_plugin_browserbookmark_browseBrowserBookmark
  • flowlauncher_plugin_browserbookmark_guideMessage03
  • flowlauncher_plugin_browserbookmark_engine_detection_invalid
  • flowlauncher_plugin_browserbookmark_engine_detection_chromium
  • flowlauncher_plugin_browserbookmark_engine_detection_firefox
  • flowlauncher_plugin_browserbookmark_engine_detection_select_directory

I can draft the Spanish strings for these keys.


🛠️ Refactor suggestion

🧩 Analysis chain

Crowdin path and migration note (namespace change).

Since files moved to Plugin.BrowserBookmarks/…, update Crowdin config/mapping to preserve translation memory and avoid resetting locale progress.


🏁 Script executed:

#!/bin/bash
set -euo pipefail
# Locate Crowdin config and check paths referencing old BrowserBookmark location
fd -a 'crowdin*.yml' .
rg -n 'BrowserBookmark' -g 'crowdin*.yml' -n || true
rg -n 'BrowserBookmarks' -g 'crowdin*.yml' -n || true

Length of output: 30


🏁 Script executed:

#!/bin/bash
set -euo pipefail
file="/home/jailuser/git/crowdin.yml"
echo "FILE: $file"
rg -n --hidden -S 'BrowserBookmark|BrowserBookmarks|Plugin.BrowserBookmarks|Flow.Launcher.Plugin.BrowserBookmarks' "$file" || true
echo "---- START FILE CONTENT ----"
sed -n '1,240p' "$file" || true
echo "---- END FILE CONTENT ----"

Length of output: 311


Update Crowdin config to include plugin Language files (preserve translation memory)

crowdin.yml at repo root currently only maps Languages/en.xaml and Properties/Resources.resx; it does not match Plugins/Flow.Launcher.Plugin.BrowserBookmarks/Languages/*, so translations for moved plugin files will be lost.

  • Edit crowdin.yml (repo root) to add a glob or explicit source for plugin language files, e.g.:
    • source: "Plugins/**/Languages/en.xaml"
      translation: "/%original_path%/%two_letters_code%.xaml"
      or add an explicit entry for Plugins/Flow.Launcher.Plugin.BrowserBookmarks/Languages/en.xaml.
  • Run a Crowdin import/dry-run to confirm existing translations map to the new paths.
🤖 Prompt for AI Agents
In Plugins/Flow.Launcher.Plugin.BrowserBookmarks/Languages/es.xaml lines 2-33,
the review notes Crowdin config at the repo root currently only maps
Languages/en.xaml and Properties/Resources.resx so plugin language files (like
this one) aren’t included and translations will be lost; fix by editing
crowdin.yml to add a source mapping for plugin language files (either a glob
such as Plugins/**/Languages/en.xaml with a translation pattern like
/%original_path%/%two_letters_code%.xaml or an explicit mapping for
Plugins/Flow.Launcher.Plugin.BrowserBookmarks/Languages/en.xaml), commit the
updated crowdin.yml, then run a Crowdin import or dry-run to verify existing
translations map to the new paths.

Comment on lines +66 to +84
private void TriggerDebouncedReload()
{
// Reset the timer to fire after 2 seconds.
// This prevents multiple reloads if a browser writes to the file several times in quick succession.
lock (_lock)
{
_debounceTimer.Change(2000, Timeout.Infinite);
}
}

public void Dispose()
{
_debounceTimer?.Dispose();
foreach (var watcher in _watchers)
{
watcher.EnableRaisingEvents = false;
watcher.Dispose();
}
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Timer dispose race can throw ObjectDisposedException.

TriggerDebouncedReload may call Change while Dispose is freeing the timer. Guard with a disposed flag and lock; also unsubscribe watcher events before dispose.

@@
-    // Timer to debounce file change events
-    private Timer _debounceTimer;
+    // Timer to debounce file change events
+    private Timer _debounceTimer;
     private readonly object _lock = new();
+    private volatile bool _disposed;
@@
     private void TriggerDebouncedReload()
     {
         // Reset the timer to fire after 2 seconds.
         // This prevents multiple reloads if a browser writes to the file several times in quick succession.
         lock (_lock)
         {
-            _debounceTimer.Change(2000, Timeout.Infinite);
+            if (_disposed) return;
+            _debounceTimer?.Change(2000, Timeout.Infinite);
         }
     }
@@
     public void Dispose()
     {
-        _debounceTimer?.Dispose();
-        foreach (var watcher in _watchers)
-        {
-            watcher.EnableRaisingEvents = false;
-            watcher.Dispose();
-        }
+        lock (_lock)
+        {
+            _disposed = true;
+            _debounceTimer?.Change(Timeout.Infinite, Timeout.Infinite);
+            _debounceTimer?.Dispose();
+            _debounceTimer = null;
+        }
+        foreach (var watcher in _watchers)
+        {
+            watcher.EnableRaisingEvents = false;
+            watcher.Changed -= OnFileChanged;
+            watcher.Created -= OnFileChanged;
+            watcher.Deleted -= OnFileChanged;
+            watcher.Renamed -= OnFileRenamed;
+            watcher.Dispose();
+        }
     }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
private void TriggerDebouncedReload()
{
// Reset the timer to fire after 2 seconds.
// This prevents multiple reloads if a browser writes to the file several times in quick succession.
lock (_lock)
{
_debounceTimer.Change(2000, Timeout.Infinite);
}
}
public void Dispose()
{
_debounceTimer?.Dispose();
foreach (var watcher in _watchers)
{
watcher.EnableRaisingEvents = false;
watcher.Dispose();
}
}
// Timer to debounce file change events
private Timer _debounceTimer;
private readonly object _lock = new();
private volatile bool _disposed;
private void TriggerDebouncedReload()
{
// Reset the timer to fire after 2 seconds.
// This prevents multiple reloads if a browser writes to the file several times in quick succession.
lock (_lock)
{
if (_disposed) return;
_debounceTimer?.Change(2000, Timeout.Infinite);
}
}
public void Dispose()
{
lock (_lock)
{
_disposed = true;
_debounceTimer?.Change(Timeout.Infinite, Timeout.Infinite);
_debounceTimer?.Dispose();
_debounceTimer = null;
}
foreach (var watcher in _watchers)
{
watcher.EnableRaisingEvents = false;
watcher.Changed -= OnFileChanged;
watcher.Created -= OnFileChanged;
watcher.Deleted -= OnFileChanged;
watcher.Renamed -= OnFileRenamed;
watcher.Dispose();
}
}
🤖 Prompt for AI Agents
In
Plugins/Flow.Launcher.Plugin.BrowserBookmarks/Services/BookmarkWatcherService.cs
around lines 66-84, TriggerDebouncedReload can call _debounceTimer.Change while
Dispose is running and freeing the timer, causing ObjectDisposedException; fix
by adding a private bool _disposed flag, wrap accesses to _debounceTimer and
_disposed in the same lock used currently so TriggerDebouncedReload checks
_disposed before calling Change, and in Dispose acquire the lock, set _disposed
= true, stop/unsubscribe all watcher events (remove event handlers) before
setting EnableRaisingEvents = false and disposing them, then safely dispose
_debounceTimer and release the lock so no Change can race with disposal.

Comment on lines +10 to +11
using System.Net.Http;
using System.Security.Cryptography;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Enable HTTP decompression (fix gzipped HTML parsing).

Without AutomaticDecompression, many pages return gzipped/br content and HTML parsing will see binary. Enable it at handler construction.

Apply this diff:

 using System.Net.Http;
+using System.Net;

 ...
-        var handler = new HttpClientHandler { AllowAutoRedirect = true };
+        var handler = new HttpClientHandler
+        {
+            AllowAutoRedirect = true,
+            AutomaticDecompression = DecompressionMethods.GZip | DecompressionMethods.Deflate | DecompressionMethods.Brotli
+        };
         _httpClient = new HttpClient(handler);

Also applies to: 54-59

🤖 Prompt for AI Agents
In Plugins/Flow.Launcher.Plugin.BrowserBookmarks/Services/FaviconService.cs
around lines 10-11 (and also apply same fix to lines 54-59), the HttpClient is
created without enabling AutomaticDecompression, causing gzipped/br responses to
be left compressed and parsed as binary; update the HttpClientHandler
construction to set AutomaticDecompression = DecompressionMethods.GZip |
DecompressionMethods.Deflate | DecompressionMethods.Brotli (or include Brotli if
available) and pass that handler into the HttpClient so responses are
automatically decompressed before HTML parsing.

Comment on lines +24 to +29
private readonly string _faviconCacheDir;
private readonly HttpClient _httpClient;
private readonly LocalFaviconExtractor _localExtractor;

private readonly ConcurrentDictionary<string, Task<string?>> _ongoingFetches = new();

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Decouple in‑flight fetch dedup from caller tokens; also make keys case‑insensitive and cancel on dispose.

A single caller cancellation currently cancels the shared in‑flight task for the same authority, causing spurious misses for other callers. Use a service‑level CTS and case‑insensitive keys; cancel outstanding work on Dispose.

Apply this diff:

     private readonly Settings _settings;
     private readonly string _faviconCacheDir;
     private readonly HttpClient _httpClient;
     private readonly LocalFaviconExtractor _localExtractor;
+    private readonly CancellationTokenSource _disposeCts = new();

-    private readonly ConcurrentDictionary<string, Task<string?>> _ongoingFetches = new();
+    private readonly ConcurrentDictionary<string, Task<string?>> _ongoingFetches =
+        new(StringComparer.OrdinalIgnoreCase);

 ...
-    private Task<string?> GetFaviconFromWebAsync(Uri url, CancellationToken token)
+    private Task<string?> GetFaviconFromWebAsync(Uri url, CancellationToken token)
     {
         if (url is null || (url.Scheme != "http" && url.Scheme != "https"))
         {
             return Task.FromResult<string?>(null);
         }
         var authority = url.GetLeftPart(UriPartial.Authority);
-        return _ongoingFetches.GetOrAdd(authority, key => FetchAndCacheFaviconAsync(new Uri(key), token));
+        // Decouple the shared fetch task from per-caller tokens
+        return _ongoingFetches.GetOrAdd(authority,
+            key => FetchAndCacheFaviconAsync(new Uri(key), CancellationToken.None));
     }

 ...
-        using var overallCts = CancellationTokenSource.CreateLinkedTokenSource(token);
+        using var overallCts = CancellationTokenSource.CreateLinkedTokenSource(_disposeCts.Token);
         overallCts.CancelAfter(TimeSpan.FromSeconds(10));
         var linkedToken = overallCts.Token;

And dispose cancellation:

     public void Dispose()
     {
+        _disposeCts.Cancel();
         _httpClient.Dispose();
         GC.SuppressFinalize(this);
     }

Also applies to: 101-109, 128-131, 421-425

🤖 Prompt for AI Agents
In Plugins/Flow.Launcher.Plugin.BrowserBookmarks/Services/FaviconService.cs
around lines 24-29 (and also update occurrences at 101-109, 128-131, 421-425):
the current in‑flight fetch dictionary and task creation use caller-provided
CancellationTokens and case-sensitive keys which lets one caller cancel the
shared task and treats equivalent hosts as distinct; change the
ConcurrentDictionary to use a case‑insensitive string comparer, introduce a
service‑level CancellationTokenSource (_cts) used for all in‑flight operations,
and ensure when starting fetch tasks you only use the service token (or link
caller token for local cancellation without cancelling the shared task) so
individual caller cancellation does not cancel the shared fetch; when creating
linked tokens use CancellationTokenSource.CreateLinkedTokenSource(service token,
caller token) only for per‑caller waiters while the underlying fetch uses just
the service token, and on Dispose cancel the service _cts and clear/cancel
outstanding work so all in‑flight fetches stop promptly.

Comment on lines +192 to +206
private async Task<FetchResult> FetchAndProcessUrlAsync(Uri faviconUri, CancellationToken token)
{
var tempPath = Path.GetTempFileName();
try
{
_context.API.LogDebug(nameof(FaviconService), $"Attempting to fetch favicon: {faviconUri}");
using var request = new HttpRequestMessage(HttpMethod.Get, faviconUri);
var response = await _httpClient.SendAsync(request, HttpCompletionOption.ResponseContentRead, token);

if (!response.IsSuccessStatusCode)
{
_context.API.LogDebug(nameof(FaviconService), $"Fetch failed for {faviconUri} with status code {response.StatusCode}");
File.Delete(tempPath);
return default;
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Guard against oversized downloads (DoS/memory).

Add a simple max size check before reading image bodies.

Apply this diff:

     private async Task<FetchResult> FetchAndProcessUrlAsync(Uri faviconUri, CancellationToken token)
     {
         var tempPath = Path.GetTempFileName();
         try
         {
             _context.API.LogDebug(nameof(FaviconService), $"Attempting to fetch favicon: {faviconUri}");
             using var request = new HttpRequestMessage(HttpMethod.Get, faviconUri);
             var response = await _httpClient.SendAsync(request, HttpCompletionOption.ResponseContentRead, token);
 
             if (!response.IsSuccessStatusCode)
             {
                 _context.API.LogDebug(nameof(FaviconService), $"Fetch failed for {faviconUri} with status code {response.StatusCode}");
                 File.Delete(tempPath);
                 return default;
             }
+            // Hard cap (1 MB) to avoid pathological images
+            const long MaxImageBytes = 1_000_000;
+            if (response.Content.Headers.ContentLength is long len && len > MaxImageBytes)
+            {
+                _context.API.LogDebug(nameof(FaviconService), $"Skipping {faviconUri}: content too large ({len} bytes).");
+                return default;
+            }

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents
In Plugins/Flow.Launcher.Plugin.BrowserBookmarks/Services/FaviconService.cs
around lines 192-206, the fetch logic reads the response body without size
limits which can lead to DoS/memory issues; add a max allowed bytes constant
(e.g. 1-2MB) and enforce it before and while saving the response: first check
response.Content.Headers.ContentLength and fail early if it exceeds the limit,
then when copying the response stream to the temp file read and write in chunks
while tracking total bytes and abort/delete/return default if the running total
exceeds the limit or cancellation is requested; ensure temp file is deleted and
response/streams are disposed on early exit and that appropriate debug logging
is added for size-rejection cases.

Comment on lines +51 to +64
// Try to find the default-release profile first, which is the most common case.
var profileSectionMatch = Regex.Match(iniContent, @"\[Profile[^\]]+\]\s*Name=default-release[\s\S]+?Path=([^\r\n]+)[\s\S]+?Default=1", RegexOptions.IgnoreCase);

// Fallback to any default profile.
if (!profileSectionMatch.Success)
{
profileSectionMatch = Regex.Match(iniContent, @"\[Profile[^\]]+\][\s\S]+?Path=([^\r\n]+)[\s\S]+?Default=1", RegexOptions.IgnoreCase);
}

// Fallback to the first available profile if no default is marked.
if (!profileSectionMatch.Success)
{
profileSectionMatch = Regex.Match(iniContent, @"\[Profile[^\]]+\][\s\S]+?Path=([^\r\n]+)");
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

🧩 Analysis chain

Regex patterns could miss valid profiles if Default=1 is on a separate line.

The current regex patterns assume that Path= and Default=1 are in sequence without other profile sections in between. However, INI format allows these properties to appear on separate lines with other properties interspersed. This could cause valid default profiles to be missed.

Let me verify the typical structure of Firefox profiles.ini:


🏁 Script executed:

#!/bin/bash
# Check if any Firefox profiles.ini files exist and display their structure
fd -H "profiles.ini" | head -5 | while read -r file; do
  echo "=== Content of $file ==="
  cat "$file" | head -30
  echo ""
done

Length of output: 0


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Locate the file
echo "Searching for FirefoxProfileFinder.cs..."
file_path=$(fd -HI --hidden --no-ignore-vcs "FirefoxProfileFinder.cs" || true)
if [ -z "$file_path" ]; then
  echo "FILE_NOT_FOUND"
  exit 0
fi

echo "Found: $file_path"
echo "---- Full file (first 400 lines) ----"
sed -n '1,400p' "$file_path" || true

echo ""
echo "---- Context around lines 40-80 ----"
sed -n '40,80p' "$file_path" || true

echo ""
echo "---- Lines containing 'Regex' or 'Profile' ----"
rg -n --hidden --no-ignore-vcs "Regex|\\[Profile|profiles.ini|Path=|Default=" "$file_path" || true

Length of output: 5543


Restrict regex to a single [Profile] section and accept Default=1 before or after Path.

Current regexes in Plugins/Flow.Launcher.Plugin.BrowserBookmarks/Services/FirefoxProfileFinder.cs (≈ lines 52–63) can fail when Default=1 appears before Path or incorrectly pair a Path from one profile with Default=1 from another because [\s\S]+? can cross section boundaries.

  • Fix: parse profiles.ini by profile sections (split on ^[Profile) and inspect each section's Path/Default/Name/IsRelative (recommended), or update the regex to forbid matching across the next "[Profile" and allow Default=1 to appear either before or after Path.

@dcog989
Copy link
Contributor Author

dcog989 commented Sep 24, 2025

A very noticeable problem is that users will lose their settings and caches of old Bookmark plugin since you change the namespace.

The only issue I see here is that users would lose manually entered custom browsers. Caches would be rebuilt so that would be transparent to the user. Plus, they have a new option to choose either way.

I can easily switch this back to BrowserBookmark if you think that is the best solution?? Which is the path of least pain??

@Jack251970
Copy link
Member

@jjw24 Any ideas about this?

@jjw24
Copy link
Member

jjw24 commented Sep 24, 2025

  • Keep the namespace, plugin name, plugin ID and project filename as is please (as Jack mentioned, there are indeed automations built around them)
  • Cater for any transition that is needed for user's data to move from old to new plugin (haven't had time to look at PR, this is an assumption on my part). Transition should be seamless, for example if users don't look they wouldn't even notice they have been moved to this new one.
  • how much different is this to the old plugin, is it a complete rewrite?

@dcog989
Copy link
Contributor Author

dcog989 commented Sep 25, 2025

@jjw24 - 👍

is it a complete rewrite?

Yes, completely refactored, rewritten. The only thing I have reused / updated is XAML/.cs

@dcog989
Copy link
Contributor Author

dcog989 commented Sep 25, 2025

I'm closing this and opening a new PR to replace the existing BrowserBookmark plugin.

Note: I have incorporated all feedback from this PR in to the new code.

@dcog989 dcog989 closed this Sep 25, 2025
@dcog989 dcog989 deleted the BrowserBookmarks-next branch September 25, 2025 04:15
@jjw24 jjw24 removed this from the 2.1.0 milestone Sep 25, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants